Bug 24641 - pthread_rwlock_rdlock can deadlock
Summary: pthread_rwlock_rdlock can deadlock
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 3.3-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-threads (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2001-01-25 17:50 UTC by earl_chew
Modified: 2006-06-05 01:30 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description earl_chew 2001-01-25 17:50:00 UTC
The man page for pthread_rwlock_rdlock() says:

   A thread may hold multiple concurrent read locks.  If so,
   pthread_rwlock_unlock() must be called once for each lock obtained.

I tried this out after reading Bill Lewis' article on Usenet:
http://x64.deja.com/[ST_rn=ps]/getdoc.xp?AN=573590892&search=thread&CONTEXT=980444523.1181483044&HIT_CONTEXT=980444500.1181483040&HIT_NUM=11&hitnum=3

Bill wrote:

    Consider: T1 gets a read lock. T2 wants a write
    lock & blocks, waiting for T1. Now T1 wants a read
    lock again. Because writers have priority, T1 will
    now block, waiting for T2.

My test program prints the following, then gets stuck:

    > ./a.out
    Attempt to acquire read lock first
    Acquired read lock first
    Attempt to acquire write lock
    Attempt to acquire read lock second

Examining the source code (uthread_rwlock.c,v 1.5) it is pretty
clear why this deadlock occurs (as Bill Lewis predicts).

How-To-Repeat: #include <pthread.h>
#include <stdio.h>
#include <unistd.h>
#include <assert.h>

static pthread_rwlock_t rwlock1 = PTHREAD_RWLOCK_INITIALIZER;

static volatile int wrStarted;

void * wrfunc(void *)
{
  printf("Attempt to acquire write lock\n");
  assert(pthread_rwlock_wrlock(&rwlock1) == 0);
  printf("Acquired write lock\n");

  assert(pthread_rwlock_unlock(&rwlock1) == 0);

  return 0;
}

static volatile int rdStarted;

void * rdfunc(void *)
{
  printf("Attempt to acquire read lock first\n");
  assert(pthread_rwlock_rdlock(&rwlock1) == 0);
  printf("Acquired read lock first\n");

  rdStarted = 1;

  while (wrStarted == 0)
      sleep(1);

  printf("Attempt to acquire read lock second\n");
  assert(pthread_rwlock_rdlock(&rwlock1) == 0);
  printf("Acquired read lock second\n");

  assert(pthread_rwlock_unlock(&rwlock1) == 0);
  assert(pthread_rwlock_unlock(&rwlock1) == 0);

  return 0;
}

int
main(int, char**)
{
  pthread_t wrt;
  pthread_t rdt;
  pthread_attr_t a;

  assert(pthread_rwlock_init(&rwlock1, 0) == 0);

  assert(pthread_attr_init(&a) == 0);

  assert(pthread_create(&rdt, &a, rdfunc, NULL) == 0);

  while (rdStarted == 0)
      sleep(1);

  assert(pthread_create(&wrt, &a, wrfunc, NULL) == 0);

  assert(pthread_join(wrt, 0) == 0);
  assert(pthread_join(rdt, 0) == 0);

  assert(pthread_rwlock_destroy(&rwlock1) == 0);

  assert(pthread_detach(wrt) == 0);
  assert(pthread_detach(rdt) == 0);

  return 0;
}
Comment 1 Jason Evans freebsd_committer freebsd_triage 2001-12-17 19:33:22 UTC
Responsible Changed
From-To: freebsd-bugs->jasone

I'll take a look at this.
Comment 2 Jason Evans freebsd_committer freebsd_triage 2002-05-11 23:22:34 UTC
Responsible Changed
From-To: jasone->freebsd-bugs
Comment 3 Kris Kennaway freebsd_committer freebsd_triage 2003-07-13 02:36:26 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-threads

Assign to threads mailing list
Comment 4 earl_chew 2004-01-05 19:11:40 UTC
The Thodes wrote:
> First, Earl, GCC objected to your test program.

Apologies. I compiled this as a C++ program (g++) hence the missing
parameter names.

> The patch given is needed to make it compile (in my case, at least).

Yes, your patch will fill in the "junk" names, and allow the C
compiler to parse the source successfully.

> Also, it (your test program)
> doesn't even get to trying to acquire the second read lock.  It hangs 
> trying to acquire the write lock.  The output is provided in this (long) 
> follow-up.

Oops... there was a cut-and-paste error. Please look at the amended
program below.

Earl
--
#include <pthread.h>
#include <stdio.h>
#include <unistd.h>
#include <assert.h>

static pthread_rwlock_t rwlock1 = PTHREAD_RWLOCK_INITIALIZER;

static volatile int wrStarted;

void * wrfunc(void *unused)
{
   printf("Attempt to acquire write lock\n");
   assert(pthread_rwlock_wrlock(&rwlock1) == 0);
   printf("Acquired write lock\n");

   assert(pthread_rwlock_unlock(&rwlock1) == 0);

   return 0;
}

static volatile int rdStarted;

void * rdfunc(void *unused)
{
   printf("Attempt to acquire read lock first\n");
   assert(pthread_rwlock_rdlock(&rwlock1) == 0);
   printf("Acquired read lock first\n");

   rdStarted = 1;

   while (wrStarted == 0)
       sleep(1);

   printf("Attempt to acquire read lock second\n");
   assert(pthread_rwlock_rdlock(&rwlock1) == 0);
   printf("Acquired read lock second\n");

   assert(pthread_rwlock_unlock(&rwlock1) == 0);
   assert(pthread_rwlock_unlock(&rwlock1) == 0);

   return 0;
}

int
main(int argc, char **argv)
{
   pthread_t wrt;
   pthread_t rdt;
   pthread_attr_t a;

   assert(pthread_rwlock_init(&rwlock1, 0) == 0);

   assert(pthread_attr_init(&a) == 0);

   assert(pthread_create(&rdt, &a, rdfunc, NULL) == 0);

   while (rdStarted == 0)
       sleep(1);

   assert(pthread_create(&wrt, &a, wrfunc, NULL) == 0);

   assert(pthread_join(wrt, 0) == 0);
   assert(pthread_join(rdt, 0) == 0);

   assert(pthread_rwlock_destroy(&rwlock1) == 0);

   assert(pthread_detach(wrt) == 0);
   assert(pthread_detach(rdt) == 0);

   return 0;
}
Comment 5 earl_chew 2004-01-05 22:21:00 UTC
aspiesrule@mcleodusa.net wrote:
> Same results, except for the fact that stepping thru your code in GDB yields 
> a segfault in KERNEL32!IsBadWritePtr.

It must be the holidays! Ok, I've added the missing line at the
beginning of wrfunc().

Earl
--
#include <pthread.h>
#include <stdio.h>
#include <unistd.h>
#include <assert.h>

static pthread_rwlock_t rwlock1 = PTHREAD_RWLOCK_INITIALIZER;

static volatile int wrStarted;

void * wrfunc(void *unused)
{
   wrStarted = 1;

   printf("Attempt to acquire write lock\n");
   assert(pthread_rwlock_wrlock(&rwlock1) == 0);
   printf("Acquired write lock\n");

   assert(pthread_rwlock_unlock(&rwlock1) == 0);

   return 0;
}

static volatile int rdStarted;

void * rdfunc(void *unused)
{
   printf("Attempt to acquire read lock first\n");
   assert(pthread_rwlock_rdlock(&rwlock1) == 0);
   printf("Acquired read lock first\n");

   rdStarted = 1;

   while (wrStarted == 0)
       sleep(1);

   printf("Attempt to acquire read lock second\n");
   assert(pthread_rwlock_rdlock(&rwlock1) == 0);
   printf("Acquired read lock second\n");

   assert(pthread_rwlock_unlock(&rwlock1) == 0);
   assert(pthread_rwlock_unlock(&rwlock1) == 0);

   return 0;
}

int
main(int argc, char **argv)
{
   pthread_t wrt;
   pthread_t rdt;
   pthread_attr_t a;

   assert(pthread_rwlock_init(&rwlock1, 0) == 0);

   assert(pthread_attr_init(&a) == 0);

   assert(pthread_create(&rdt, &a, rdfunc, NULL) == 0);

   while (rdStarted == 0)
       sleep(1);

   assert(pthread_create(&wrt, &a, wrfunc, NULL) == 0);

   assert(pthread_join(wrt, 0) == 0);
   assert(pthread_join(rdt, 0) == 0);

   assert(pthread_rwlock_destroy(&rwlock1) == 0);

   assert(pthread_detach(wrt) == 0);
   assert(pthread_detach(rdt) == 0);

   return 0;
}
Comment 6 earl_chew 2004-01-05 23:26:25 UTC
aspiesrule@mcleodusa.net wrote:
> Oops, running your new program yields an assertion failure at line 37 and a 
> core dump.  The output is as follows:
> 
> --snip--
> Attempt to acquire read lock first
> Acquired read lock first
> Attempt to acquire write lock
> Attempt to acquire read lock second
> assertion "pthread_rwlock_rdlock(&rwlock1) == 0" failed: file "test.c", line 
> 37
> Aborted (core dumped)

Right. The test is (correctly) showing an inadequacy in the rwlock
implementation.

I refer you to the original bug report:

http://www.freebsd.org/cgi/query-pr.cgi?pr=24641

and the referenced articles:

http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&oe=UTF-8&selm=slrn87nusa.rsv.kaz%40ashi.FootPrints.net
http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&oe=UTF-8&selm=38828D22.7A98%40LambdaCS.com

Earl
Comment 7 Craig Rodrigues freebsd_committer freebsd_triage 2006-06-05 01:28:27 UTC
State Changed
From-To: open->closed

Problem does not occur in libpthread in FreeBSD 6.x. 

Your testcase needs to be modified slightly. 
If you call pthread_join(), 
then if you call pthread_detach() later on, pthread_detach 
may return EINVAL, because the thread may already be 
terminated and detached, so you can't detach it. 


#include <pthread.h> 
#include <stdio.h> 
#include <unistd.h> 
#include <assert.h> 
#include <errno.h> 

static pthread_rwlock_t rwlock1 = PTHREAD_RWLOCK_INITIALIZER; 

static volatile int wrStarted; 

void * wrfunc(void *unused) 
{ 
wrStarted = 1; 

printf("Attempt to acquire write lockn"); 
assert(pthread_rwlock_wrlock(&rwlock1) == 0); 
printf("Acquired write lockn"); 

assert(pthread_rwlock_unlock(&rwlock1) == 0); 

return 0; 
} 

static volatile int rdStarted; 

void * rdfunc(void *unused) 
{ 
printf("Attempt to acquire read lock firstn"); 

assert(pthread_rwlock_rdlock(&rwlock1) == 0); 
printf("Acquired read lock firstn"); 

rdStarted = 1; 

while (wrStarted == 0) 
sleep(1); 

printf("Attempt to acquire read lock secondn"); 
assert(pthread_rwlock_rdlock(&rwlock1) == 0); 
printf("Acquired read lock secondn"); 

assert(pthread_rwlock_unlock(&rwlock1) == 0); 
assert(pthread_rwlock_unlock(&rwlock1) == 0); 

return 0; 
} 

int 
main(int argc, char **argv) 
{ 
pthread_t wrt; 
pthread_t rdt; 
pthread_attr_t a; 
int ret; 

assert(pthread_rwlock_init(&rwlock1, 0) == 0); 

assert(pthread_attr_init(&a) == 0); 

assert(pthread_create(&rdt, &a, rdfunc, NULL) == 0); 

while (rdStarted == 0) 
sleep(1); 

assert(pthread_create(&wrt, &a, wrfunc, NULL) == 0); 

assert(pthread_join(wrt, 0) == 0); 
assert(pthread_join(rdt, 0) == 0); 

assert(pthread_rwlock_destroy(&rwlock1) == 0); 

if ((ret = pthread_detach(wrt)) != 0) { 
if (ret == EINVAL) { 
printf("wrt already detachedn"); 
} 
} 

if ((ret = pthread_detach(rdt)) != 0) { 
if (errno == EINVAL) { 
printf("rdt already detachedn"); 
} 
}  
return 0; 
}