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

JackClient: Lock before killing client notification thread #889

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

DeviLaxmii
Copy link
Contributor

The client notification thread locks a mutex before reading/writing
from/to the socket. If the thread is killed while the lock is taken it
would leave the mutex dangling.

To avoid this take the mutex and then kill the client notification thread.

Signed-off-by: Laxmi Devi [email protected]

@DeviLaxmii
Copy link
Contributor Author

DeviLaxmii commented Aug 15, 2022

Hello,

I was able to reproduce the issue mentioned in : #395
With the below script

#!/bin/sh
export JACK_PROMISCUOUS_SERVER=jack; ulimit -r80 -l100000
a=0

while [ $a -lt 60 ]
do
   echo $a
   a=`expr $a + 1`
   aplay -v -d3  -Djack /dev/urandom &
done

Kindly consider the patch which resolves the issue.

The client notification thread locks a mutex before reading/writing
from/to the socket. If the thread is killed while the lock is taken it
would leave the mutex dangling.

To avoid this take the mutex and then kill the client notification thread.


Signed-off-by: Laxmi Devi <[email protected]>
@DeviLaxmii DeviLaxmii force-pushed the jack_blocks_during_client_close_request branch from 8032ee2 to e933e28 Compare August 15, 2022 09:40
@ReinholdH
Copy link

Thanks for the commit.
After searching for hours and almost next to give up, I found bug #395 and this commit #889.
Please add this commit to the next Jack update 1.9.22.
In particular on Windows it is important when opening/closing Jack clients frequently.
Thanks.

@ReinholdH
Copy link

This commit did not make it to Jack 1.9.22 despite it looks really simple.
Has there been any reason why?
Is this fix not complete?
To me this issue is a show stopper to effectively use Jack on Windows.
Thanks

@falkTX
Copy link
Member

falkTX commented Feb 3, 2023

thread sync always looks simple but never really is.
I need some time to do some testing of my own, for whch I dont have at the moment.
there were blockers on macOS and in waf that prevented jack2 from even being built at all, that took priority.
will see about other stuff like this once later

@ReinholdH
Copy link

Thanks for your immediate response. I fully understand. Good luck for fixing such stuff.

@ReinholdH
Copy link

Just to confirm, I have checked the issue again with 1.9.22. The locking still happens with Jack 1.9.22.

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.

3 participants