rncbc / qtractor

Qtractor - An Audio/MIDI multi-track sequencer
https://qtractor.org
GNU General Public License v2.0
514 stars 90 forks source link

Possible crasher found? #91

Closed schnitzeltony closed 7 years ago

schnitzeltony commented 7 years ago

While working on 'magic' MIDI import dialog I saw a possible source of qtractor crashing with segfault. Consider this:

  1. Open 'Add track'
  2. Select 'MIDI': That calls qtractorPluginList::setChannels several times and finally with channels = 2 / iFlags = qtractorPluginList::MidiTrack. That creates by qtractorMidiManager::createMidiManager which adds a newly created MidiManager to the session by calling qtractorSession::addMidiManager.
  3. Cancel Dialog and confirm to discard changes. The track is deleted which deletes PluginList which calls qtractorPluginList::setChannels(0,0). The MidiManager is removed by qtractorSession::removeMidiManager and then deleted.

Problem: In qtractorAudioEngine::process which is called by Jack's worker thread the MidiManagers are iterated (qTractorAudioEngine.cpp line 923). If this done while changing list (2./3.) the list might be in unfinished condition -> boom. Seems to me MidiManager-list has to be made thread safe...

rncbc commented 7 years ago

yes, it might, but rather unlikely to happen so often, at least on ix86 architectures, where pointer changes are assumed atomic; other cpu-architectures where that is not guaranteed, then yes, one may say it's marginally unsafe.

you see, like any other qtractorList node, qtractorMidiManager removal doesn't quite end in calling on operator delete() for that matter: it just moves into a free-list to be actually delete'd later on session close.

so even though an item is removed from the list, it remains still a valid pointer target for most if not all situations--potentially, the worst that may happen, is possibly one-time glitch/incomplete list traversing, but rather quite unlikely to segfault due to freed/delete'd memory hazard pointer access.

hth.