Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JACK support #1926

Conversation

theGreatWhiteShark
Copy link
Contributor

@theGreatWhiteShark theGreatWhiteShark commented Jan 1, 2024

Probably due to some upstream JACK bugs (not sure) Hydrogen does occassionally segfault when stopping the JackAudioDriver. Turns out JACK has a number of issues with thread joining. During teardown of the driver we must ensure it does not do any communication, like locking the AudioEngine or enqueuing a message in our Logger. Otherwise the thread calling jack_client_close might segfault.

Now, the callback of the AudioEngine checks its state twice. Once without locking (in order to return when it is not ready yet/anymore) and another time while locked to properly check the state. It is a little dirty. But I do not see what else we can do from within the Hydrogen code base

Fixes #1929


Previous text:

Lately we have quite a number of reports of Hydrogen crashing during shutdown or song export all related to our JackAudioDriver (#1902, #1907, #1867).

I have to admit I have seen those for quite a while now on my device. But since they are occurring very rarely (like one out of 20) and neither my Hydrogen nor JACK are in a "production" setup, I didn't bothered after giving it a first, inconclusive shot. But the JACK version I use (1.9.21) seems to tickle down the package repos and more and more users get the same issues as well.

I wrote a Go-based integration test starting up Hydrogen hundreds of times and killing it via OSC right away. This way I'm now able to reproduce the issue.

So far it looks like this is an upstream problem and code of JACK crashes when attempting to tear down the JackAudioDriver. We might need a way to protect against crashing driver code. But I'm far from understand what is going on and still need to check and read into a couple of things first.

@theGreatWhiteShark theGreatWhiteShark marked this pull request as draft January 1, 2024 20:14
to reproduce crash on teardown when using JACK as audio driver.

Test is not done yet. It works properly on non-zero exit codes but not on crashes (which is what we encounter here)
jackTearDown integration test can now handle crashes of Hydrogen and shows - thanks to our crash reporter - the stack trace
@theGreatWhiteShark
Copy link
Contributor Author

Another problem: Occassionally upstream JACK code used in our JackAudioDriver gets trapped in a deadlock when stopping the driver (see jackaudio/jack2#395).

This does not occur as often as the crashes on my machine. But is still annoying.

to prevent false positives occurring in case the OSC QUIT message gets send before Hydrogen has properly started up
probably due to some upstream JACK bugs (not sure) Hydrogen does occassionally segfault when stopping the `JackAudioDriver`. Turns out JACK has a number of issues with thread joining. During teardown of the driver we must ensure it does not do any communication, like locking the `AudioEngine` or enqueuing a message in our `Logger`. Otherwise the thread calling `jack_client_close` might segfault.

Now, the callback of the `AudioEngine` checks its state twice. Once without locking (in order to `return` when it is not ready yet/anymore) and another time while locked to properly check the state. It is a little dirty. But I do not see what else we can do from within the Hydrogen code base
@theGreatWhiteShark theGreatWhiteShark marked this pull request as ready for review January 4, 2024 12:33
@theGreatWhiteShark theGreatWhiteShark merged commit d9f4037 into hydrogen-music:releases/1.2 Jan 5, 2024
1 check passed
@theGreatWhiteShark theGreatWhiteShark deleted the phil-integration-tests branch January 5, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant