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

thread_init callback is called multiple times #104

Open
HadrienG2 opened this issue Sep 3, 2019 · 7 comments
Open

thread_init callback is called multiple times #104

HadrienG2 opened this issue Sep 3, 2019 · 7 comments

Comments

@HadrienG2
Copy link

Not sure why...

@wmedrano
Copy link
Member

wmedrano commented Sep 4, 2019

Thanks for filing the issue. I observed this on my system using libjack2.
I'm not sure what happens for libjack.

The crate's documentation says it should be called only once, but this was just copied from the JACK API Docs: http://jackaudio.org/files/docs/html/group__ClientCallbacks.html#gad5a6904292f5c9790223c18aeab202bf

I'm not sure what the proper behavior is, but the issue should be reported to the JACK developers.

@HadrienG2
Copy link
Author

HadrienG2 commented Sep 4, 2019

Oh, wait, I think I got it.

One headliner feature of jack2, which I also use, is that it supports multiple audio processing threads. What I did not realize is that for process isolation reasons, it may actually need to spawn multiple audio threads per client in order to achieve this result.

If that is the issue, then it's just a matter of clarifying the thread_init_callback documentation.

EDIT: And indeed, on my machine, printing std::thread::current().id() in the thread_init callback yields

Audio thread ThreadId(2) is ready.
Audio thread ThreadId(3) is ready.
Audio thread ThreadId(4) is ready.

@HadrienG2 HadrienG2 changed the title It seems that the thread_init callback is called three times thread_init callback is called multiple times Sep 4, 2019
@HadrienG2
Copy link
Author

Note that if there are multiple audio threads and they can work concurrently, some aspects of the jack-rs API may need to change towards multi-thread-safety. For example, NotificationHandler and ProcessHandler would likely need to become Sync.

@wmedrano
Copy link
Member

Since JACK can have many implementations (and has 2) we should follow the spec. Since the spec is not specific on this, we can't make guarantees about future implementations so its safer to require Sync.

We should also update the example to use crossbeam::channel for communication since std channels are not Sync on the receiver side.

@piegamesde
Copy link
Contributor

piegamesde commented Nov 30, 2019

Requiring Sync is pretty bad, since it breaks all stateful process handlers (using Arc is not an option because its spinlocks are not thread safe). Is it really possible to call the processing callback not only from different threads, but concurrently? If I understand https://github.com/jackaudio/jackaudio.github.com/wiki/WalkThrough_User_ClientThreads correctly, there are only 2 threads, one for notifications and one for processing (btw the link to the documentation above is broken)

@HadrienG2
Copy link
Author

using Arc is not an option because its spinlocks are not thread safe

Can you clarify what you mean by that ?

If I understand https://github.com/jackaudio/jackaudio.github.com/wiki/WalkThrough_User_ClientThreads correctly, there are only 2 threads, one for notifications and one for processing (btw the link to the documentation above is broken)

This is indeed consistent with one of the last footnotes at the end of this jack1 vs jack2 comparison, but not with the fact that my thread_init_callback was called from three different threads above. Not sure what to make of all this honestly, it would be great if a jack2 dev could clarify the situation here.

@piegamesde
Copy link
Contributor

* using Arc is not an option because its blocking wait is not realtime thread safe

But never mind that, I just found out that Mutex has a non-blocking try_lock which works in a RT context.

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

No branches or pull requests

3 participants