I wrote a recording program using pulseaudio's API. version 2.1 I found that pa_mainloop_quit can't make pa_mainloop_run exit. I did a little digging on this problem, and I think I have found this bug. In pa_mainloop_quit, pa_mainloop_wakeup is called to make pa_mainloop_poll unblocked from select. However, In pa_mainloop_wakeup, only if m->state == STATE_POLLING, this wake up signal is sent to pa_mainloop_poll. In pa_mainloop_poll, checking if m->quit is set and setting m->state to STATE_POLLING is not protected. So, in this case, in multi-thread program, it is possible that Mainloop-thread: pa_mainloop_poll checked m->quit is set to 0, m->state != STATE_POLLING Controlling-thread: pa_mainloop_quit->pa_mainloop_wakeup checked m->state != STATE_POLLING, failed sending wake up signal Mainloop-thread: pa_mainloop_polling set m->state = STATE_POLLING and go on calling pa_poll, and everything stops here. There is on solution: pa_mainloop_wakeup doesn't check if m->state == STATE_POLLING, it just send wake up signal, so pa_mainloop_polling will receive this signal Or protect the m->quit checking and m->state setting section in pa_mainloop_poll and protect pa_mainloop_quit with on mutex, make these two API won't run in mixed way. To work around this bug, I have to call pa_mainloop_quit multiple times to make sure pa_mainloop_run is quit. Hope this bug can be fixed sooner.
Thanks for the report. I think the right solution is what you proposed first: just don't check the mainloop state in pa_mainloop_wakeup(). There's already an old patch for that, which has been buried due to lack of reviewer time. I'll ping the mailing list about that. I'll attach the patch also here on this bug, so that you can check if it fixes your problem if you want.
Actually the patch depends on two other patches. I'll attach all three here.
Created attachment 69659 [details] [review] patch 1/3: Change wakeup_requested type
Created attachment 69660 [details] [review] patch 2/3: Remove redundant wakeup_pipe validity checks
Created attachment 69661 [details] [review] patch 3/3: Write to the wakeup pipe unconditionally
I have been busy these days. I'll help to test these patches when I am available. It seems that these patches can fix this bug. :) (In reply to comment #2) > Actually the patch depends on two other patches. I'll attach all three here.
(In reply to comment #1) > Thanks for the report. I think the right solution is what you proposed > first: just don't check the mainloop state in pa_mainloop_wakeup(). There's > already an old patch for that, which has been buried due to lack of reviewer > time. I'll ping the mailing list about that. > > I'll attach the patch also here on this bug, so that you can check if it > fixes your problem if you want. Hi Tanu, I have tested in my program for a short time. These patches can fix the pulse_mainloop_quit()'s problem. Thanks for your help!
The brokenness of pa_mainloop_wakeup() has now been fixed: http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=872f56dc7ec43cb1511ef0d25c1736a373b3c2e2 Yupeng, unfortunately I've come to the conclusion (after getting feedback from Arun) that it's actually not supported to call pa_mainloop_quit() from a different thread. It will probably continue working quite well in practice in 3.0 (better than 2.x now that the pa_mainloop_wakeup() bug has been fixed), but it will be completely broken in 4.0, because the pa_mainloop_wakeup() call will be removed from pa_mainloop_quit(). The thing is that pa_mainloop_quit() is not a thread-safe function, because it directly modifies the mainloop state which is supposed to be accessed only from the thread where the mainloop runs. The only thread-safe function in the pa_mainloop API is pa_mainloop_wakeup(). If you really need to quit the mainloop from a different thread, create an IO event (using a pipe, for example) that you trigger from the other thread.
As another note, the pa_threaded_mainloop seems to be closer to what you'd like to be using. Any reason to not use that?
(In reply to comment #9) > As another note, the pa_threaded_mainloop seems to be closer to what you'd > like to be using. Any reason to not use that? How to create an IO event to quit pa_mainloop_run() ? Can you give me some example codes? I designed my program based on the callback mechanism of pa_mainloop, so I didn't use threaded-mainloop.
(In reply to comment #10) > I designed my program based on the callback mechanism of pa_mainloop, so I > didn't use threaded-mainloop. The threaded mainloop is also going to end up calling those callbacks and it fits your model of usage a lot better, so if you can, you really should look into using it.
(In reply to comment #11) > (In reply to comment #10) > > I designed my program based on the callback mechanism of pa_mainloop, so I > > didn't use threaded-mainloop. > > The threaded mainloop is also going to end up calling those callbacks and it > fits your model of usage a lot better, so if you can, you really should look > into using it. Thanks for your advise. I have considered using threaded-mainloop, but there's one thing made me give up. My program is a real-time recording program, other threads are set to RT thread. There isn't an API of threaded-mainloop to set it to run as a RT thread. So I decided to use pa_mainloop and wrap it into a thread class which I can control all the thread preferences. It's my fault that I didn't read the source code carefully. There's just one step close to the success. The only way I can think of is to register a signal handler in the same thread of pa_mainloop_run, in this handler, pa_mainloop_quit is called, and trigger this signal in other thread.
(In reply to comment #12) [...] > Thanks for your advise. > I have considered using threaded-mainloop, but there's one thing made me > give up. > My program is a real-time recording program, other threads are set to RT > thread. > There isn't an API of threaded-mainloop to set it to run as a RT thread. [...] You can do this in the context state callback before you actually connect the stream, which is well before the audio work actually begins.
(In reply to comment #13) > (In reply to comment #12) > [...] > > Thanks for your advise. > > I have considered using threaded-mainloop, but there's one thing made me > > give up. > > My program is a real-time recording program, other threads are set to RT > > thread. > > There isn't an API of threaded-mainloop to set it to run as a RT thread. > [...] > > You can do this in the context state callback before you actually connect > the stream, which is well before the audio work actually begins. I think the best way to do so, is to send a special message, for example "quit", to m->wakeup_pipe[1], in the poll function, when this special message is received, m->quit is set to true, then poll quit cleanly. In this model, pa_mainloop_quit() just "tells" pa_mainloop_run() to quit, the former API won't modify any private data of PaMainloop. B.T.W signal handler is not elegant to make mainloop quit. it's just a hack method.
(In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #12) > > [...] > > > Thanks for your advise. > > > I have considered using threaded-mainloop, but there's one thing made me > > > give up. > > > My program is a real-time recording program, other threads are set to RT > > > thread. > > > There isn't an API of threaded-mainloop to set it to run as a RT thread. > > [...] > > > > You can do this in the context state callback before you actually connect > > the stream, which is well before the audio work actually begins. > > I think the best way to do so, is to send a special message, for example > "quit", to m->wakeup_pipe[1], in the poll function, when this special > message is received, m->quit is set to true, then poll quit cleanly. > > In this model, pa_mainloop_quit() just "tells" pa_mainloop_run() to quit, > the former API won't modify any private data of PaMainloop. I'm not very enthusiastic about doing this. > B.T.W signal handler is not elegant to make mainloop quit. it's just a hack > method. Did you consider Arun's suggestion of using pa_threaded_mainloop and setting up the scheduling from the context state callback? Signals certainly are an ugly way to implement the wakeup (I'm assuming that by "signal handler" you're talking about the same stuff that "man 2 signal" is talking about). Luckily, that isn't necessary, even if you don't use pa_threaded_mainloop. I'll answer your earlier question: (In reply to comment #10) > How to create an IO event to quit pa_mainloop_run() ? > Can you give me some example codes? > > I designed my program based on the callback mechanism of pa_mainloop, so I > didn't use threaded-mainloop. pa_mainloop *mainloop = pa_mainloop_new(); pa_mainloop_api *mainloop_api = pa_mainloop_get_api(mainloop); int pipefds[2] = { -1, -1 }; pipe(pipefds); pa_io_event *quit_event = mainloop_api->io_new(pipefds[0], PA_IO_EVENT_INPUT, quit_event_cb, userdata); Now you can write() to pipefds[1] from any thread, and quit_event_cb() will get called in the thread where the mainloop runs.
(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > (In reply to comment #12) > > > [...] > > > > Thanks for your advise. > > > > I have considered using threaded-mainloop, but there's one thing made me > > > > give up. > > > > My program is a real-time recording program, other threads are set to RT > > > > thread. > > > > There isn't an API of threaded-mainloop to set it to run as a RT thread. > > > [...] > > > > > > You can do this in the context state callback before you actually connect > > > the stream, which is well before the audio work actually begins. > > > > I think the best way to do so, is to send a special message, for example > > "quit", to m->wakeup_pipe[1], in the poll function, when this special > > message is received, m->quit is set to true, then poll quit cleanly. > > > > In this model, pa_mainloop_quit() just "tells" pa_mainloop_run() to quit, > > the former API won't modify any private data of PaMainloop. > > I'm not very enthusiastic about doing this. > > > B.T.W signal handler is not elegant to make mainloop quit. it's just a hack > > method. > > Did you consider Arun's suggestion of using pa_threaded_mainloop and setting > up the scheduling from the context state callback? > > Signals certainly are an ugly way to implement the wakeup (I'm assuming that > by "signal handler" you're talking about the same stuff that "man 2 signal" > is talking about). Luckily, that isn't necessary, even if you don't use > pa_threaded_mainloop. I'll answer your earlier question: > > (In reply to comment #10) > > How to create an IO event to quit pa_mainloop_run() ? > > Can you give me some example codes? > > > > I designed my program based on the callback mechanism of pa_mainloop, so I > > didn't use threaded-mainloop. > > pa_mainloop *mainloop = pa_mainloop_new(); > pa_mainloop_api *mainloop_api = pa_mainloop_get_api(mainloop); > int pipefds[2] = { -1, -1 }; > pipe(pipefds); > pa_io_event *quit_event = mainloop_api->io_new(pipefds[0], > PA_IO_EVENT_INPUT, quit_event_cb, userdata); > > Now you can write() to pipefds[1] from any thread, and quit_event_cb() will > get called in the thread where the mainloop runs. Thank you very much!
(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > (In reply to comment #12) > > > [...] > > > > Thanks for your advise. > > > > I have considered using threaded-mainloop, but there's one thing made me > > > > give up. > > > > My program is a real-time recording program, other threads are set to RT > > > > thread. > > > > There isn't an API of threaded-mainloop to set it to run as a RT thread. > > > [...] > > > > > > You can do this in the context state callback before you actually connect > > > the stream, which is well before the audio work actually begins. > > > > I think the best way to do so, is to send a special message, for example > > "quit", to m->wakeup_pipe[1], in the poll function, when this special > > message is received, m->quit is set to true, then poll quit cleanly. > > > > In this model, pa_mainloop_quit() just "tells" pa_mainloop_run() to quit, > > the former API won't modify any private data of PaMainloop. > > I'm not very enthusiastic about doing this. > > > B.T.W signal handler is not elegant to make mainloop quit. it's just a hack > > method. > > Did you consider Arun's suggestion of using pa_threaded_mainloop and setting > up the scheduling from the context state callback? > > Signals certainly are an ugly way to implement the wakeup (I'm assuming that > by "signal handler" you're talking about the same stuff that "man 2 signal" > is talking about). Luckily, that isn't necessary, even if you don't use > pa_threaded_mainloop. I'll answer your earlier question: > > (In reply to comment #10) > > How to create an IO event to quit pa_mainloop_run() ? > > Can you give me some example codes? > > > > I designed my program based on the callback mechanism of pa_mainloop, so I > > didn't use threaded-mainloop. > > pa_mainloop *mainloop = pa_mainloop_new(); > pa_mainloop_api *mainloop_api = pa_mainloop_get_api(mainloop); > int pipefds[2] = { -1, -1 }; > pipe(pipefds); > pa_io_event *quit_event = mainloop_api->io_new(pipefds[0], > PA_IO_EVENT_INPUT, quit_event_cb, userdata); > > Now you can write() to pipefds[1] from any thread, and quit_event_cb() will > get called in the thread where the mainloop runs. This works perfectly! Thank you very much!
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.