From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8224 invoked by alias); 20 Sep 2002 02:40:16 -0000 Mailing-List: contact pthreads-win32-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: pthreads-win32-owner@sources.redhat.com Received: (qmail 8215 invoked from network); 20 Sep 2002 02:40:14 -0000 Received: from unknown (HELO digit.ise.canberra.edu.au) (137.92.140.41) by sources.redhat.com with SMTP; 20 Sep 2002 02:40:14 -0000 Received: from ise.canberra.edu.au (localhost.localdomain [127.0.0.1]) by digit.ise.canberra.edu.au (8.11.6/8.11.6) with ESMTP id g8K2e9o13211; Fri, 20 Sep 2002 12:40:09 +1000 Message-ID: <3D8A8A88.3000109@ise.canberra.edu.au> Date: Thu, 19 Sep 2002 19:40:00 -0000 From: Ross Johnson Organization: University of Canberra, Information Sciences and Engineering User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.9) Gecko/20020513 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Michael Johnson CC: pthreads-win32 discussion list Subject: Re: Deadlock interaction between pthread_cond_check_need_init.c and pthread_cond_destroy.c References: <3D89F90A.2030805@maine.rr.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2002/txt/msg00099.txt.bz2 Michael, I've applied your patch and committed it to CVS. I'll write a test for it later. Many thanks for this. Ross Michael Johnson wrote: > When two different threads exist, and one is attempting to destroy a > condition variable while the other is attempting to initialize a > condition variable that was created with PTHREAD_COND_INITIALIZER, a > deadlock can occur: > > Thread A Thread B > Enters ptw32_cond_check_need_init > > > Enters pthread_cond_destroy > EnterCriticalSection(&ptw32_cond_test_init_lock) > > (now holds ptw32_cond_test_init_lock) > EnterCriticalSection(&ptw32_cond_list_lock) > Enters pthread_cond_init > Determines that condvar is static initialized > EnterCriticalSection(&ptw32_cond_list_lock) > EnterCriticalSection(&ptw32_cond_test_init_lock) > (now waiting to enter ptw32_cond_list_lock) > (now waiting to enter ptw32_cond_test_init_lock) > deadlocked > > > It appears from reading the code and from a patch that I made that the > following change to pthread_cond_destroy appears to fix this: > | > | > > |{| > | pthread_cond_t cv;| > | int result = 0, result1 = 0, result2 = 0;| > || > | /*| > | * Assuming any race condition here is harmless.| > | */| > | if (cond == NULL| > | || *cond == NULL)| > | {| > | return EINVAL;| > | }| > || > | if (*cond != PTHREAD_COND_INITIALIZER)| > | {| > | EnterCriticalSection(&ptw32_cond_list_lock);| > || > | cv = *cond;| > || > | /*| > | * Close the gate; this will synchronize this thread with| > | * all already signaled waiters to let them retract their| > | * waiter status - SEE NOTE 1 ABOVE!!!| > | */| > | if (sem_wait(&(cv->semBlockLock)) != 0)| > | {| > | return errno;| > | }| > || > | /*| > | * !TRY! lock mtxUnblockLock; try will detect busy condition| > | * and will not cause a deadlock with respect to concurrent| > | * signal/broadcast.| > | */| > | if ((result = pthread_mutex_trylock(&(cv->mtxUnblockLock))) > != 0)| > | {| > | (void) sem_post(&(cv->semBlockLock));| > | return result;| > | }| > || > | /*| > | * Check whether cv is still busy (still has waiters)| > | */| > | if (cv->nWaitersBlocked > cv->nWaitersGone)| > | {| > | if (sem_post(&(cv->semBlockLock)) != 0)| > | {| > | result = errno;| > | }| > | result1 = pthread_mutex_unlock(&(cv->mtxUnblockLock));| > | result2 = EBUSY;| > | }| > | else| > | {| > | /*| > | * Now it is safe to destroy| > | */| > | *cond = NULL;| > || > | if (sem_destroy(&(cv->semBlockLock)) != 0)| > | {| > | result = errno;| > | }| > | if (sem_destroy(&(cv->semBlockQueue)) != 0)| > | {| > | result1 = errno;| > | }| > | if ((result2 = > pthread_mutex_unlock(&(cv->mtxUnblockLock))) == 0)| > | {| > | result2 = pthread_mutex_destroy(&(cv->mtxUnblockLock));| > | }| > || > | /* Unlink the CV from the list */| > || > | if (ptw32_cond_list_head == cv)| > | {| > | ptw32_cond_list_head = cv->next;| > | }| > | else| > | {| > | cv->prev->next = cv->next;| > | }| > || > | if (ptw32_cond_list_tail == cv)| > | {| > | ptw32_cond_list_tail = cv->prev;| > | }| > | else| > | {| > | cv->next->prev = cv->prev;| > | }| > || > | (void) free(cv);| > | }| > || > | LeaveCriticalSection(&ptw32_cond_list_lock);| > | }| > | else| > | {| > | /*| > | * See notes in ptw32_cond_check_need_init() above also.| > | */| > | EnterCriticalSection(&ptw32_cond_test_init_lock);| > || > | /*| > | * Check again.| > | */| > | if (*cond == PTHREAD_COND_INITIALIZER)| > | {| > | /*| > | * This is all we need to do to destroy a statically| > | * initialised cond that has not yet been used > (initialised).| > | * If we get to here, another thread waiting to initialise| > | * this cond will get an EINVAL. That's OK.| > | */| > | *cond = NULL;| > | }| > | else| > | {| > | /*| > | * The cv has been initialised while we were waiting| > | * so assume it's in use.| > | */| > | result = EBUSY;| > | }| > || > | LeaveCriticalSection(&ptw32_cond_test_init_lock);| > | }| > || > | return ((result != 0) ? result : ((result1 != 0) ? result1 : > result2));| > |}| >