Ticket #1147 (accepted defect)

Opened 2 years ago

Last modified 3 months ago

Multithreaded spectrum can cause race->crash with PCM audio provider

Reported by: nielsm Owned by: nielsm
Priority: normal Milestone: 3.0.0
Component: Audio Version: 2.1.8
Severity: crash Keywords: pcmwav
Cc: Platform: All
Sub Component:

Description

The PCM WAV audio provider is not thread safe. It works most of the time, when it doesn't have to re-map the view of the file, but if two threads attempt to access parts of the audio that would lie in different parts of the file, a race condition can occur where one thread begins obtaining audio, reaches past the mapping check code and is ready/begins copying from the mapping. Simultaneously, another thread requests audio outside the current mapping, which causes the mapping code to tear down the current mapping and set up a new one. As a result, the copying in the first thread suddenly happens from an invalid memory location and the program crashes.

The only case where this can currently happen in Aegisub is the audio spectrum, since it is OpenMP multithreaded.

The proper solution is to allow the file mapping code to manage multiple mappings at a time and protect those mappings with multi-reader locks, possibly also adding some LRU logic to determine which mapping to tear down when the time comes.

This ties into #934, "Factor file mappings out of PCM reader".

Change History

comment:1 Changed 20 months ago by nielsm

  • Status changed from new to accepted
  • Owner set to nielsm

comment:2 Changed 20 months ago by nielsm

Most reliably way to reproduce:
Only sensible on 32 bit builds since builds with pointers larger than 32 bits use 2 GB views rather than 256 MB views of the file by default.
Open audio file larger than 256 MB, ideally, larger than 512 MB to have multiple "breaking points".
Enable spectrum mode.
In an 48 kHz 16 bit stereo file, the breaks happen every approx. 22m45s, so scroll the audio so some audio before and after a break should be visible at the same time. Then the crash might happen.

comment:3 Changed 20 months ago by nielsm

  • Milestone changed from 2.1.9 to 2.2.0

The fix for this is will be too complex to be worth implementing for 2.1.9.
Pushing forward.

comment:4 Changed 20 months ago by nielsm

Workarounds:

  • Use a 64 bit build of Aegisub.
  • Don't use spectrum mode.
  • Resample the audio file to be smaller (e.g. resample stereo to mono, 48 kHz to 24 kHz...)
  • Use a different audio provider (e.g. use an MP3 or Ogg Vorbis file instead so the FFMS2 provider is used) - note that this will require large amounts of memory for long audio files

comment:5 Changed 18 months ago by verm

  • Milestone changed from 2.2.0 to 3.0.0

Bump 2.2.0 tickets to milestone:3.0.0 (2.2.0 is becoming 3.0.0)

comment:6 Changed 13 months ago by nielsm

The base issue is no longer relevant, as the ADR code is not multithreaded.

The underlying issue with raw audio data not having handles, scopes or anything similar, is still present though and could come up again at a later time.

comment:7 Changed 3 months ago by Plorkyeran

This is still reproducable - some of the audio players read from the audio provider on a background thread, so scrolling to the break point while audio is playing can cause a crash.

Note: See TracTickets for help on using tickets.