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
>
