hi
I don't know whether we understand...Nobody in patch see pthread_t.
Only os_core_unix.c implementation where problem occured.
In alsa_dev.c is using pthread_t but this is "problem" how driver has been
written.
This issue is not only in ALSA dev...this problem can occur in any thread
created with os_core_unix.c implementation..
Yes no one promise that new thread will not be start before
pj_thread_create() or pthread_create() end. But IMHO should be promise that
new thread have set thread ID in system variable when is running because
if you have following situation:
thread1: starting method "thread_create"
thread1: creating new thread 2
sheduler: switching to thread 2
thread2 : calling some method e.g.:pj_thread_get_os_handle (pj_thread_this());
sheduler: switching to thread 1
thread1: setting thread ID
thread1: end method "thread create"
Will not be probably work as you wish.
regards
> my point:
> Don't need change so much. Only the code (for example ALSA) which need
> to access "shared" pthread_t need to take care of this issue.It is
> programer's duty to handle shared data race conditions.
>
> As you know most code don't care pthread_t. And currently
> pj_thread_create() works the same way as pthread_create().No one
> promise new thread
> must not be startup before pj_thread_create() or pthread_create() return.
> Adding waiting mutex to pj_thread_create is a big behavior changes for
> programs which heavily use dynamic thread creation.
>
> regards,
> Gang
>
> On Wed, Mar 2, 2011 at 5:40 PM, Peter Lukac <p.lukac at emtest.sk> wrote:
> > thanks for reply,
> >
> > Nobody see struct pj_thread_t except concrete inplementation like:
> > os_core_unix.c, os_core_win32.c ..etc
> >
> > variable of type pthread_t is visible only in os_core_unix.c
> > implementation therefore is wait mutex only in unix implementation for
> > pj_thread_create()
> >
> > If i want use flag PJ_THREAD_SUSPENDED i have to overwrite all common
> > code where is pj_thread_create() called for right behaviour in other
> > threads and i don't think this is problem in windows or symbian or
> > others platform (i don't try it)...this is pthread "problem" where
> > thread can be run before as function pthread_create() set new thread ID.
> >
> > I don't think the this solution can have performance impact because new
> > thread is unlock immediately when pthread_create() end.
> >
> >
> > regards
> >
> > On Wednesday, March 02, 2011 08:31:01 am Gang Liu wrote:
> >> I would suggest set flag PJ_THREAD_SUSPENDED when creating ALSA dev
> >> thread. And start it after pj_thread_create() return.
> >>
> >> Add wait mutex to pj_thread_create() isn't a good solution because not
> >> all threads will access to
> >> shared pthread_t .
> >> Some applications may require new threads start up asap.
> >>
> >> regards,
> >> Gang
> >>
> >> On Thu, Feb 17, 2011 at 12:48 AM, Peter Lukac <p.lukac at emtest.sk> wrote:
> >> > hello all,
> >> >
> >> > I think i'm found little problem when is creating new thread with
> >> > function pj_thread_create() with implementation in os_core_unix.c
> >> > throught pthread.
> >> >
> >> > when you look at line 587 in os_core_unix.c :
> >> >
> >> > rc = pthread_create( &rec->thread, &thread_attr, &thread_main, rec);
> >> >
> >> > This method create thread and run it with function:
> >> > static void *thread_main(void *param)
> >> > and set pthread_t structure.
> >> >
> >> > Problem occured when thread is running before function
> >> > pthread_create() set new thread ID to pthread_t structure
> >> > (rec->thread) which is passed like argument to thread function. In
> >> > my situation in ALSA sound driver in thread for microphone is setting
> >> > thread priority by tread ID :
> >> >
> >> > alsa_dev.c
> >> > line: 430
> >> >
> >> > thid = (pthread_t*) pj_thread_get_os_handle (pj_thread_this());
> >> >
> >> > this command return invalid threadID because function pthread_create()
> >> > still don't set new thread ID... (OS scheduler don't switch to thread
> >> > from which was pthread_create() called )
> >> >
> >> > This problem can occur on all threads in which on beginning is calling
> >> > some function with "this thread ID".
> >> >
> >> > Solution should be using "suspended_mutex" from structure pj_thread_t
> >> > or add new wait mutex...
> >> >
> >> > i made little patch which should works. If somebody have better
> >> > solution please send me...
> >> >
> >> > thanks
> >> >
> >> > here is patch:
> >> >
> >> > *** pjproject-1.8.5/pjlib/src/pj/os_core_unix.c 2010-09-24
> >> > 09:49:32.000000000 +0200
> >> > --- pjproject-1.8.5_alsa_test/pjlib/src/pj/os_core_unix.c
> >> > 2011-02-16 17:41:19.000000000 +0100
> >> > ***************
> >> > *** 58,64 ****
> >> > pj_uint32_t signature1;
> >> > pj_uint32_t signature2;
> >> >
> >> > ! pj_mutex_t *suspended_mutex;
> >> >
> >> > #if defined(PJ_OS_HAS_CHECK_STACK) && PJ_OS_HAS_CHECK_STACK!=0
> >> > pj_uint32_t stk_size;
> >> > --- 58,65 ----
> >> > pj_uint32_t signature1;
> >> > pj_uint32_t signature2;
> >> >
> >> > ! pj_mutex_t *suspended_mutex;
> >> > ! pj_mutex_t *wait_mutex;
> >> >
> >> > #if defined(PJ_OS_HAS_CHECK_STACK) && PJ_OS_HAS_CHECK_STACK!=0
> >> > pj_uint32_t stk_size;
> >> > ***************
> >> > *** 480,485 ****
> >> > --- 482,491 ----
> >> > pj_assert(!"Thread TLS ID is not set (pj_init() error?)");
> >> > }
> >> >
> >> > + /* Wait while function pthread_create() finish */
> >> > + pj_mutex_lock(rec->wait_mutex);
> >> > + pj_mutex_unlock(rec->wait_mutex);
> >> > +
> >> > /* Check if suspension is required. */
> >> > if (rec->suspended_mutex) {
> >> > pj_mutex_lock(rec->suspended_mutex);
> >> > ***************
> >> > *** 556,561 ****
> >> > --- 562,578 ----
> >> > pj_assert(rec->suspended_mutex == NULL);
> >> > }
> >> >
> >> > + /*
> >> > + Create wait mutex because thread can be run before as
> >> > + pthread_create() set thread ID
> >> > + */
> >> > + rc = pj_mutex_create_simple(pool, NULL, &rec->wait_mutex);
> >> > + if (rc != PJ_SUCCESS) {
> >> > + return rc;
> >> > + }
> >> > +
> >> > + /* lock wait mutex..new thread will be wait while
> >> > pthread_create() end*/ + pj_mutex_lock(rec->wait_mutex);
> >> >
> >> > /* Init thread attributes */
> >> > pthread_attr_init(&thread_attr);
> >> > ***************
> >> > *** 587,592 ****
> >> > --- 604,612 ----
> >> > return PJ_RETURN_OS_ERROR(rc);
> >> > }
> >> >
> >> > + /* unlock new thread... now thread ID is set*/
> >> > + pj_mutex_unlock(rec->wait_mutex);
> >> > +
> >> > *ptr_thread = rec;
> >> >
> >> > PJ_LOG(6, (rec->obj_name, "Thread created"));
> >> > ***************
> >> > *** 700,705 ****
> >> > --- 720,731 ----
> >> > p->suspended_mutex = NULL;
> >> > }
> >> >
> >> > + /* Destroy mutex used to wait thread */
> >> > + if (p->wait_mutex) {
> >> > + pj_mutex_destroy(p->wait_mutex);
> >> > + p->wait_mutex = NULL;
> >> > + }
> >> > +
> >> > return PJ_SUCCESS;
> >> > }
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >> > _______________________________________________
> >> > Visit our blog: http://blog.pjsip.org
> >> >
> >> > pjsip mailing list
> >> > pjsip at lists.pjsip.org
> >> > http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
> >>
> >> _______________________________________________
> >> Visit our blog: http://blog.pjsip.org
> >>
> >> pjsip mailing list
> >> pjsip at lists.pjsip.org
> >> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
> >
> > _______________________________________________
> > Visit our blog: http://blog.pjsip.org
> >
> > pjsip mailing list
> > pjsip at lists.pjsip.org
> > http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>
> _______________________________________________
> Visit our blog: http://blog.pjsip.org
>
> pjsip mailing list
> pjsip at lists.pjsip.org
> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
