[BUG] macOS: a crash report pop-up appears on exiting the app #680
Comments
jonathanperret
added a commit
to jonathanperret/ayab-desktop
that referenced
this issue
Jul 24, 2024
The current code spawns a dedicated thread to handle playing sound effects. This is unnecessary as the `simpleaudio` API already is non-blocking, i.e. calling `wave_obj.play()` returns immediately while the sound plays in the background (see https://simpleaudio.readthedocs.io/en/latest/capabilities.html#asynchronous-interface). At the worst, loading the sound file, which is synchronous, could block the calling thread for a few milliseconds but that happens only once and is unlikely to be an issue. The `AudioWorker.play` method does have a `blocking` argument that is supposed to support blocking the caller until the sound has finished playing, but this is never used in the current codebase. But more conclusively, it turns out that due to an oversight, practically no code currently executes on the audio player thread. The `AudioWorker` object is assigned to a new thread, but because the `AudioPlayer.play` method calls `__worker.play` without going through a signal, the `AudioWorker.play` method actually executes on the thread that called `AudioPlayer.play`. In practice this is the UI thread, because `audio.play` is called through a signal on `SignalReceiver`, which lives on that thread. Thus, the time-sensitive engine thread is already isolated from any slowness in reading or playing audio. This commit reduces the `AudioPlayer` class to a plain Python object whose only responsibilities are to load and play the application's sound effects. It also simplifies the file loading logic by removing the direct call to the `wave` library in favor of the `from_wave_file` helper method offered by `simpleaudio`, which in practice uses the exact same call to the `wave` library. A notable benefit of removing this thread is that the application no longer crashes with an `abort` call upon exit (issue AllYarnsAreBeautiful#680), caused by improper termination of the audio thread.
dl1com
pushed a commit
that referenced
this issue
Jul 25, 2024
The current code spawns a dedicated thread to handle playing sound effects. This is unnecessary as the `simpleaudio` API already is non-blocking, i.e. calling `wave_obj.play()` returns immediately while the sound plays in the background (see https://simpleaudio.readthedocs.io/en/latest/capabilities.html#asynchronous-interface). At the worst, loading the sound file, which is synchronous, could block the calling thread for a few milliseconds but that happens only once and is unlikely to be an issue. The `AudioWorker.play` method does have a `blocking` argument that is supposed to support blocking the caller until the sound has finished playing, but this is never used in the current codebase. But more conclusively, it turns out that due to an oversight, practically no code currently executes on the audio player thread. The `AudioWorker` object is assigned to a new thread, but because the `AudioPlayer.play` method calls `__worker.play` without going through a signal, the `AudioWorker.play` method actually executes on the thread that called `AudioPlayer.play`. In practice this is the UI thread, because `audio.play` is called through a signal on `SignalReceiver`, which lives on that thread. Thus, the time-sensitive engine thread is already isolated from any slowness in reading or playing audio. This commit reduces the `AudioPlayer` class to a plain Python object whose only responsibilities are to load and play the application's sound effects. It also simplifies the file loading logic by removing the direct call to the `wave` library in favor of the `from_wave_file` helper method offered by `simpleaudio`, which in practice uses the exact same call to the `wave` library. A notable benefit of removing this thread is that the application no longer crashes with an `abort` call upon exit (issue #680), caused by improper termination of the audio thread.
|
Addressed in #681 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Environment
AYAB software version: 1.0.0-dev
Computer/OS: macOS 14.2 on M1
Describe the bug
The AYAB app causes a crash report window ("AYAB quit unexpectedly") to pop up after quitting.
To Reproduce
Steps to reproduce the behavior:
fbs freeze, not running from source withfbs run.Quititem from theFilemenu in the AYAB windowQuititem from theAYABmenu in the macOS menu barCmd+Qshortcut while AYAB is the active applicationIf you don't see a crash report window, please check the following before marking the bug as fixed:
~/Library/Logs/DiagnosticReportsfolder where the crash reports are stored may help, or rebooting. When in doubt, I found it helps to keep a known-affected build of AYAB around so that you can run it and check that the crash report window still appears.Expected behavior
No crash report window should pop up when exiting the app.
Additional context
This is more or less a re-opening of #640 , which I thought I had fixed with #670 but evidently my testing was not thorough enough, hence the detailed testing instructions above.
Here is the beginning of one crash report seen with the build referenced above.
The text was updated successfully, but these errors were encountered: