NSInternalInconsistencyException in PQ2 and SQ3 when using external MT-32 device

[tsoliman]

When using an external MT-32 device, I get a crash on launch of certain SCI games.

Note that you don't need an external midi device to reproduce this, you can use the Apple DLS Software Synthesizer.

Steps to reproduce in macOS Big Sur:

• Start with a fresh scummvm ini file
• Add the games without any game-specific audio settings
• In the global settings (not the game-specific ones) set both the preferred audio device and the MT-32 device to the external midi device (or to the Apple DLS Software Synthesizer if you don't have one). Because they match, it will tread this device as an MT-32 and not GM.
• Set the "True Roland MT-32" checkbox and unset the "Roland GS Device" checkbox
• Start SQ3 or PQ2
• Crash

Here's the console output from the crash

2021-09-19 18:02:18.737 scummvm[80902:1722619] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'nextEventMatchingMask should only be called from the Main Thread!'
*** First throw call stack:
(
	0   CoreFoundation                      0x00007fff2065783b __exceptionPreprocess + 242
	1   libobjc.A.dylib                     0x00007fff2038fd92 objc_exception_throw + 48
	2   AppKit                              0x00007fff22de2516 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 4389
	3   libSDL2-2.0.0.dylib                 0x00000001083b9c36 Cocoa_PumpEventsUntilDate + 88
	4   libSDL2-2.0.0.dylib                 0x00000001083b9d48 Cocoa_PumpEvents + 47
	5   libSDL2-2.0.0.dylib                 0x0000000108328d80 SDL_PumpEvents_REAL + 33
	6   libSDL2-2.0.0.dylib                 0x0000000108328e5e SDL_WaitEventTimeout_REAL + 171
	7   scummvm                             0x00000001074ba256 _ZN14SdlEventSource9pollEventERN6Common5EventE + 230
	8   scummvm                             0x00000001076ab268 _ZN6Common15EventDispatcher8dispatchEv + 216
	9   scummvm                             0x000000010748ec67 _ZN19DefaultEventManager9pollEventERN6Common5EventE + 71
	10  scummvm                             0x00000001072ea154 _ZN3Sci12EventManager15getScummVMEventEv + 180
	11  scummvm                             0x00000001072eacb3 _ZN3Sci12EventManager11getSciEventENS_12SciEventTypeE + 99
	12  scummvm                             0x00000001072f2939 _ZN3Sci9SciEngine5sleepEj + 89
	13  scummvm                             0x00000001073e64d4 _ZN3Sci15MidiPlayer_Midi5sysExEPKht + 164
	14  scummvm                             0x00000001073e31f8 _ZN3Sci15MidiPlayer_Midi13sendMt32SysExEjRN6Common18SeekableReadStreamEib + 392
	15  scummvm                             0x00000001073e29bd _ZN3Sci15MidiPlayer_Midi13sendMt32SysExEjRKNS_7SciSpanIKhEEb + 125
	16  scummvm                             0x00000001073e2e8d _ZN3Sci15MidiPlayer_Midi9initTrackERNS_7SciSpanIKhEE + 1053
	17  scummvm                             0x00000001073b9942 _ZN3Sci14MidiParser_SCI9initTrackEv + 194
	18  scummvm                             0x00000001073bc2da _ZN3Sci8SciMusic25sendMidiCommandsFromQueueEv + 298
	19  scummvm                             0x00000001073bc143 _ZN3Sci8SciMusic7onTimerEv + 51
	20  scummvm                             0x00000001073bc0f8 _ZN3Sci8SciMusic17miditimerCallbackEPv + 56
	21  scummvm                             0x00000001074a7409 _ZN19DefaultTimerManager7handlerEv + 457
	22  scummvm                             0x00000001074d0fdf _ZL13timer_handlerjPv + 47
	23  libSDL2-2.0.0.dylib                 0x0000000108361717 SDL_TimerThread + 324
	24  libSDL2-2.0.0.dylib                 0x00000001083612ab SDL_RunThread + 53
	25  libSDL2-2.0.0.dylib                 0x00000001083c7dd0 RunThread + 9
	26  libsystem_pthread.dylib             0x00007fff204e48fc _pthread_start + 224
	27  libsystem_pthread.dylib             0x00007fff204e0443 thread_start + 15
)
libc++abi: terminating with uncaught exception of type NSException
Abort trap: 6

This works fine in branch-2-2 (but not in branch-2-3 or master)

[sluicebox]

Bisected to ​https://github.com/scummvm/scummvm/commit/b67c2d72d6c40fda2d67cfaf6a8aab9f44f7cd1e from March

[criezy]

This was discussed a bit on Discord today, and to make sure this does not get lost, here is a summary.

We think the main issue is that pollEvent gets called from the audio thread. The commit found by sluice box probably caused that by moving some code execution from the main thread to the audio thread.

We can see two main ways to tackle the issue:

1. Revert the commit that introduced the regression
2. Change the Sci MidiPlayer_Midi so that when it wants to sleep in the audio thread it does not call SciEngine::sleep but instead has its own sleep function that just sleeps (and does not poll events).

We also suspect that MidiPlayer_Fb01 might have the same issue as it is also calling SciEngine::sleep.

And the issue may not be limited to calling sleep. Those players also call OSystem::updateScreen, and if that can also happen in the audio thread this would likely also be an issue.

[sluicebox]

Fixed in ecc72dce23e231b3eeb7f91a8edcb9121c37abdc