Bug report and patch: race condition for setting thread ID in unix implementation [pj_thread_create]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>



[Index of Archives]     [Asterisk Users]     [Asterisk App Development]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [Linux API]
  Powered by Linux