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

JackAudio: fix segmentation fault, revamp initialization logic #3683

Conversation

@davidebeatrici
Copy link
Member

commented May 2, 2019

Fixes #3682.

The crash happens when an invalid client is passed to jack_port_register().

This pull request changes JackAudioInit::initialize() so that it aborts the initialization process in case the client cannot be opened (e.g. when the server cannot be started due to permission issues).

bJackIsGood was set to true by init_jack(), which was called by initializeInput() and initializeOutput(). In order to be able to check whether the client is correctly initialized (bJackIsGood == true) before making the backend available to the rest of the program (e.g. settings window), this pull request merged init_jack() with JackAudioSystem() and destroy_jack() with ~JackAudioSystem().

The downside to moving the client initialization into the constructor is that it stays open (but not active) even when JACK is not used as backend, which is what currently happens with the PulseAudio backend.

In future we will improve the way audio backends are handled and fix the issue.

@gileri

This comment has been minimized.

Copy link

commented May 2, 2019

With this PR there is no freezes or crashes related to JACK, but now even with jackd listening, the JACK system does is no longer displayed as an available I/O system.

I've tested against 1.3.0rc1 and Mumble+JACK seems to work properly on my system (with a dummy backend)

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:jack-audio-segmentation-fault-null-client-fix branch from 40fecc8 to f9f1c2d May 2, 2019

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

Fixed.

From the commit message:

bJackIsGood was set to true by init_jack(), which was called by initializeInput() and initializeOutput(). In order to be able to check whether the client is correctly initialized (bJackIsGood == true) before making the backend available to the rest of the program (e.g. settings window), this commit merged init_jack() with JackAudioSystem() and destroy_jack() with ~JackAudioSystem().

The downside to moving the client initialization into the constructor is that it stays open (but not active) even when JACK is not used as backend, which is what currently happens with the PulseAudio backend.

In future we will improve the way audio backends are handled and fix the issue.

@gileri

This comment has been minimized.

Copy link

commented May 3, 2019

Thank you for such a descriptive commit :)
I can test it this evening (UTC+2).

JackAudio: fix segmentation fault, revamp initialization logic
The crash happens when an invalid client is passed to jack_port_register().

This commit changes JackAudioInit::initialize() so that it aborts the initialization process in case the client cannot be opened (e.g. when the server cannot be started due to permission issues).

bJackIsGood was set to true by init_jack(), which was called by initializeInput() and initializeOutput(). In order to be able to check whether the client is correctly initialized (bJackIsGood == true) before making the backend available to the rest of the program (e.g. settings window), this commit merged init_jack() with JackAudioSystem() and destroy_jack() with ~JackAudioSystem().

The downside to moving the client initialization into the constructor is that it stays open (but not active) even when JACK is not used as backend, which is what currently happens with the PulseAudio backend.

In future we will improve the way audio backends are handled and fix the issue.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:jack-audio-segmentation-fault-null-client-fix branch from f9f1c2d to 0ee0683 May 3, 2019

@davidebeatrici davidebeatrici changed the title JackAudio: fix segmentation fault when the client cannot be opened JackAudio: fix segmentation fault, revamp initialization logic May 3, 2019

@davidebeatrici davidebeatrici merged commit d737867 into mumble-voip:master May 4, 2019

3 of 5 checks passed

CI Build #20190503.4 failed
Details
CI (macOS) macOS failed
Details
CI (Windows MSVC_2015) Windows MSVC_2015 succeeded
Details
CI (Windows MSVC_2015_NO_PCH) Windows MSVC_2015_NO_PCH succeeded
Details
Travis CI - Pull Request Build Passed
Details
@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented May 4, 2019

Thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.