Hi all,
I am wondering if anyone can shed some light on the following predicament. I am by no means a multi-threading guru so any insight would be most appreciated.
The following are relevant excerpts from the code of an external. AFAIK the external initializes mutex and cond and spawns a secondary worker thread that deals with audio-unfriendly (xrun-causing) write operations to the wiimote and terminates it when the object is destructed waiting for the thread to join back and then destroying the mutex.
Now, if I add a bit of usleep right after the thread has been spawned as part of the constructor (as included below) the external seems very stable (e.g. cutting and pasting it as fast as keyboard allows, or in other words constructing and destructing instances of it as fast as possible does not result in a crash). Yet, when one does not use usleep right after spawning the secondary (worker) thread in the constructor, the whole thing is very crash-prone, almost as if the spawning of thread does not go well unless given adequate time to do get things all into sync, so to say, even though this makes to me no sense as the way I understand it the constructor does not move ahead until pthread_create does not return a value (which in this case I am not bothering to read).
Curiously, when not using usleep, a crash may occur right at creation time, at any point while the object exists, and even as late as during its destruction. Any ideas?
P.S. I am also including the entire file for those interested in trying it out.
Best wishes,
Ico
Relevant excerpts (in random order and incomplete to allow for greater legibility):
//struct defining the object typedef struct _wiimote { t_object x_obj; // standard pd object (must be first in struct)
...
//Creating separate threads for actions known to cause sample drop-outs pthread_t unsafe_t; pthread_mutex_t unsafe_mutex; pthread_cond_t unsafe_cond;
t_float unsafe;
...
t_float led;
...
} t_wiimote;
//constructor static void *pd_cwiid_new(t_symbol* s, int argc, t_atom *argv) { ...
x->led = 0;
// spawn threads for actions known to cause sample drop-outs
threadedFunctionParams rPars;
rPars.wiimote = x;
pthread_mutex_init(&x->unsafe_mutex, NULL);
pthread_cond_init(&x->unsafe_cond, NULL);
pthread_create( &x->unsafe_t, NULL, (void *)
&pd_cwiid_pthreadForAudioUnfriendlyOperations, (void *) &rPars);
//WHY IS THIS NECESSARY? I thought that pthread_create call will first
finish spawning thread before proceeding
usleep(100); //allow thread to sync (is there a better way to do this?)
...
}
//destructor static void pd_cwiid_free(t_wiimote* x) { if (x->connected) { pd_cwiid_doDisconnect(x); //this one has nothing to do with thread but rather disconnects the wiimote }
x->unsafe = -1; //to allow secondary thread to exit the while loop
pthread_mutex_lock(&x->unsafe_mutex);
pthread_cond_signal(&x->unsafe_cond);
pthread_mutex_unlock(&x->unsafe_mutex);
pthread_join(x->unsafe_t, NULL);
pthread_mutex_destroy(&x->unsafe_mutex);
...
}
//worker thread void pd_cwiid_pthreadForAudioUnfriendlyOperations(void *ptr) { threadedFunctionParams *rPars = (threadedFunctionParams*)ptr; t_wiimote *x = rPars->wiimote; t_float local_led = 0; t_float local_rumble = 0; unsigned char local_rpt_mode = x->rpt_mode;
while(x->unsafe > -1) {
pthread_mutex_lock(&x->unsafe_mutex);
if ((local_led == x->led) && (local_rumble == x->rumble) &&
(local_rpt_mode == x->rpt_mode)) { pthread_cond_wait(&x->unsafe_cond, &x->unsafe_mutex); }
if (local_led != x->led) {
local_led = x->led;
//do something
}
}
if (local_rumble != x->rumble) {
local_rumble = x->rumble;
//do something else
}
...
pthread_mutex_unlock(&x->unsafe_mutex);
}
pthread_exit(0);
}
//an example of how the thread is affected by the main thread void pd_cwiid_setLED(t_wiimote *x, t_floatarg f) { if (x->connected) { x->led = f;
pthread_mutex_lock(&x->unsafe_mutex);
pthread_cond_signal(&x->unsafe_cond);
pthread_mutex_unlock(&x->unsafe_mutex);
}
}
I think maybe pthread_create(&x->unsafe_t, NULL, (void *) &pd_cwiid_pthreadForAudioUnfriendlyOperations, (void *)&rPars); should be something like: pthread_create(&x->unsafe_t, NULL, pd_cwiid_pthreadForAudioUnfriendlyOperations, (void *)&rPars[this_thread]);
since rPars can't be used by any other thread, you need to make a copy for each thread.
Also the function declaration void pd_cwiid_pthreadForAudioUnfriendlyOperations(void *ptr) should be: void *pd_cwiid_pthreadForAudioUnfriendlyOperations(void *ptr)
Also nothing guarantees that a thread will be created right away. It may be that the processor has to reload its cache to do something like that, causing significant slowdown in execution speed and possible audio glitches (but not crashing).
Martin
On 2010-10-01 20:36, Ivica Ico Bukvic wrote:
Hi all,
I am wondering if anyone can shed some light on the following predicament. I am by no means a multi-threading guru so any insight would be most appreciated.
The following are relevant excerpts from the code of an external. AFAIK the external initializes mutex and cond and spawns a secondary worker thread that deals with audio-unfriendly (xrun-causing) write operations to the wiimote and terminates it when the object is destructed waiting for the thread to join back and then destroying the mutex.
Now, if I add a bit of usleep right after the thread has been spawned as part of the constructor (as included below) the external seems very stable (e.g. cutting and pasting it as fast as keyboard allows, or in other words constructing and destructing instances of it as fast as possible does not result in a crash). Yet, when one does not use usleep right after spawning the secondary (worker) thread in the constructor, the whole thing is very crash-prone, almost as if the spawning of thread does not go well unless given adequate time to do get things all into sync, so to say, even though this makes to me no sense as the way I understand it the constructor does not move ahead until pthread_create does not return a value (which in this case I am not bothering to read).
Curiously, when not using usleep, a crash may occur right at creation time, at any point while the object exists, and even as late as during its destruction. Any ideas?
P.S. I am also including the entire file for those interested in trying it out.
Best wishes,
Ico
Relevant excerpts (in random order and incomplete to allow for greater legibility):
//struct defining the object typedef struct _wiimote { t_object x_obj; // standard pd object (must be first in struct)
...
//Creating separate threads for actions known to cause sample drop-outs pthread_t unsafe_t; pthread_mutex_t unsafe_mutex; pthread_cond_t unsafe_cond;
t_float unsafe;
...
t_float led;
...
} t_wiimote;
//constructor static void *pd_cwiid_new(t_symbol* s, int argc, t_atom *argv) { ...
x->led = 0;
// spawn threads for actions known to cause sample drop-outs threadedFunctionParams rPars; rPars.wiimote = x; pthread_mutex_init(&x->unsafe_mutex, NULL); pthread_cond_init(&x->unsafe_cond, NULL); pthread_create(&x->unsafe_t, NULL, (void *) &pd_cwiid_pthreadForAudioUnfriendlyOperations, (void *)&rPars);
//WHY IS THIS NECESSARY? I thought that pthread_create call will first finish spawning thread before proceeding usleep(100); //allow thread to sync (is there a better way to do this?)
... }
//destructor static void pd_cwiid_free(t_wiimote* x) { if (x->connected) { pd_cwiid_doDisconnect(x); //this one has nothing to do with thread but rather disconnects the wiimote }
x->unsafe = -1; //to allow secondary thread to exit the while loop
pthread_mutex_lock(&x->unsafe_mutex); pthread_cond_signal(&x->unsafe_cond); pthread_mutex_unlock(&x->unsafe_mutex);
pthread_join(x->unsafe_t, NULL); pthread_mutex_destroy(&x->unsafe_mutex);
... }
//worker thread void pd_cwiid_pthreadForAudioUnfriendlyOperations(void *ptr) { threadedFunctionParams *rPars = (threadedFunctionParams*)ptr; t_wiimote *x = rPars->wiimote; t_float local_led = 0; t_float local_rumble = 0; unsigned char local_rpt_mode = x->rpt_mode;
while(x->unsafe> -1) { pthread_mutex_lock(&x->unsafe_mutex); if ((local_led == x->led)&& (local_rumble == x->rumble)&& (local_rpt_mode == x->rpt_mode)) { pthread_cond_wait(&x->unsafe_cond,&x->unsafe_mutex); }
if (local_led != x->led) { local_led = x->led; //do something } } if (local_rumble != x->rumble) { local_rumble = x->rumble; //do something else } ... pthread_mutex_unlock(&x->unsafe_mutex);} pthread_exit(0); }
//an example of how the thread is affected by the main thread void pd_cwiid_setLED(t_wiimote *x, t_floatarg f) { if (x->connected) { x->led = f;
pthread_mutex_lock(&x->unsafe_mutex); pthread_cond_signal(&x->unsafe_cond); pthread_mutex_unlock(&x->unsafe_mutex);} }
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
First of all, my apologies to all for x-posting of the original matter--I did not realize there was such a major overlap in user base on pd-list and pd-dev lists making my x-post truly redundant.
since rPars can't be used by any other thread, you need to make a copy for each thread.
This must be it! You are absolutely right as there is no guarantee rPars won't get destructed (with the end of the constructor function) before the worker thread is properly instantiated. FWIW, instead of creating a copy of rPars, I've actually gone with Robin's suggestion to use sched_yield() and have a wait condition which is cleared once the worker thread has spawned to ensure it will get the necessary data from rPars before they are destructed as follows:
void *pd_cwiid_pthreadForAudioUnfriendlyOperations(void *ptr) { threadedFunctionParams *rPars = (threadedFunctionParams*)ptr; t_wiimote *x = rPars->wiimote; t_float local_led = 0; t_float local_rumble = 0; unsigned char local_rpt_mode = x->rpt_mode;
while(x->unsafe > -1) {
pthread_mutex_lock(&x->unsafe_mutex);
if ((local_led == x->led) && (local_rumble == x->rumble) &&
(local_rpt_mode == x->rpt_mode)) { if (x->unsafe) x->unsafe = 0; //signal that the thread init is complete pthread_cond_wait(&x->unsafe_cond, &x->unsafe_mutex); }
//snip
static void *pd_cwiid_new(t_symbol* s, int argc, t_atom *argv) {
//snip
// spawn threads for actions known to cause sample drop-outs
threadedFunctionParams rPars;
rPars.wiimote = x;
pthread_mutex_init(&x->unsafe_mutex, NULL);
pthread_cond_init(&x->unsafe_cond, NULL);
pthread_create( &x->unsafe_t, NULL, (void *)
&pd_cwiid_pthreadForAudioUnfriendlyOperations, (void *) &rPars);
// wait until other thread has properly intialized so that
// rPars do not get destroyed before the thread has gotten its
// pointer information
while(x->unsafe) {
//must use as many yields as necessary as there is no
//guarantee that one will be enough
//also on Linux use sched_yield
//rather than pthread_yield
sched_yield();
}
//snip
Many thanks all for your help on this one! Hopefully the existence of this thread will help others who may be looking for similar solutions.
Best wishes,
Ico
On Sat, 2 Oct 2010, Ivica Ico Bukvic wrote:
First of all, my apologies to all for x-posting of the original matter--I did not realize there was such a major overlap in user base on pd-list and pd-dev lists making my x-post truly redundant.
It depends what you post about. I'm not on pd-dev and won't subscribe to it. I'm probably not the only one.
| Mathieu Bouchard ------------------------------ Villeray, Montréal, QC
-
Ivica Ico Bukvic -
Martin Peach -
Mathieu Bouchard