Message-ID: <50E9B35F.7030307@iki.fi>
Date: Sun, 06 Jan 2013 18:24:47 +0100
From: Antti Kantee <pooka AT iki DOT fi>
MIME-Version: 1.0
To: cygwin AT cygwin DOT com
Subject: pthread_rwlock_tryrdlock bug (with test & patch)
Content-Type: multipart/mixed; boundary="------------030102030504060305090402"
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com

--------------030102030504060305090402
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

[I'd submit this to cygwin-patches, but the web page says only 
subscribers can post to that list, so please bear with me for this 
simple issue]

Hi,

Calling pthread_rwlock_tryrdlock() twice from the same thread always 
fails with EBUSY the second time.  See attached test.c.  Replacing the 
second tryrdlock() with rdlock() makes the call succeed, leading me to 
believe there is a bug in tryrdlock().

Assuming I was looking at the correct source file, the attached patch 
should fix the issue.  I'm not sure why the lookup_reader() call was 
there in the first place; perhaps a remnant from a time when recursive 
read locking was not supported?

While looking at tryrdlock(), the handling of ULONG_MAX also seems 
wrong, as you'd probably want to return EAGAIN instead of allocating 
another reader structure.  However, that problem is arguably more of an 
academic issue and therefore I didn't touch it in my patch.

   - antti

p.s. please cc me on any responses

--------------030102030504060305090402
Content-Type: text/plain; charset=windows-1252;
 name="test.c"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="test.c"

#include <err.h>
#include <pthread.h>
#include <stdio.h>

int
main()
{
	pthread_rwlock_t lck;
	int rv;

	pthread_rwlock_init(&lck, NULL);
	if ((rv = pthread_rwlock_tryrdlock(&lck)) != 0)
		errx(1, "lock 1: %d", rv);
	if ((rv = pthread_rwlock_tryrdlock(&lck)) != 0)
		errx(1, "lock 2: %d", rv);
	printf("success\n");
}

--------------030102030504060305090402
Content-Type: text/plain; charset=windows-1252;
 name="tryrdlock.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="tryrdlock.patch"

--- src/winsup/cygwin/thread.cc.old	2012-08-17 01:34:45.000000000 +0200
+++ src/winsup/cygwin/thread.cc	2013-01-06 17:58:46.258963200 +0100
@@ -1427,7 +1427,7 @@ pthread_rwlock::tryrdlock ()
 
   mtx.lock ();
 
-  if (writer || waiting_writers || lookup_reader (self))
+  if (writer || waiting_writers)
     result = EBUSY;
   else
     {


--------------030102030504060305090402
Content-Type: text/plain; charset=us-ascii

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
--------------030102030504060305090402--