DO NOT REPLY [Bug 45455] New: rwlock sometimes allows a writer to take the lock while a reader has the lock

[bu...@apache.org]

https://issues.apache.org/bugzilla/show_bug.cgi?id=45455

           Summary: rwlock sometimes allows a writer to take the lock while
                    a reader has the lock
           Product: APR
           Version: HEAD
          Platform: PC
        OS/Version: Windows Server 2003
            Status: NEW
          Severity: normal
          Priority: P2
         Component: APR
        AssignedTo: bugs@apr.apache.org
        ReportedBy: rbarkan@websense.com


Created an attachment (id=22298)
 --> (https://issues.apache.org/bugzilla/attachment.cgi?id=22298)
A suggested fix and a test to verify

Take a look at the following lines from the apr_thread_rwlock_unlock()
(trunk/locks/win32/thread_rwlock.c):

...
146 :                   /* Nope, we must have a read lock */    
147 :                    if (rwlock->readers &&         
148 :                           ! InterlockedDecrement(&rwlock->readers) &&     
149 :                           ! SetEvent(rwlock->read_event)) {       
....

Notice that no mutex is taken during apr_thread_rwlock_unlock().

Now, consider a single reader is performing an unlock, called
InterlockedDecrement() on line 148, received 0 as a result of
InterlockedDecrement, and a context switch now happens right before the call to
SetEvent() in line 149.

Now, let's say that right after the context switch another thread tries to take
the read-lock. Here's the implementation of apr_thread_rwlock_rdlock_core():

... 
65 :                    DWORD code = WaitForSingleObject(rwlock->write_mutex,
milliseconds);    
66 :                            
67 :                     if (code == WAIT_FAILED || code == WAIT_TIMEOUT)       
68 :                     return APR_FROM_OS_ERROR(code);        
69 :                            
70 :                     /* We've successfully acquired the writer mutex, we
can't be locked    
71 :                     * for write, so it's OK to add the reader lock. The
writer mutex       
72 :                     * doubles as race condition protection for the readers
counter.        
73 :                     */     
74 :                     InterlockedIncrement(&rwlock->readers);        
75 :                            
76 :                     if (! ResetEvent(rwlock->read_event))  
77 :                            return apr_get_os_error();      
78 :
79 :                     if (! ReleaseMutex(rwlock->write_mutex))       
80 :                            return apr_get_os_error();      
81 :
82 :                    return APR_SUCCESS;     
 ...

After this thread is done taking the lock, the readers counter is equal to 1,
and the read_event is RESET.
Now, the system might context-switch back to the original thread doing an
unlock operation (at line 149) which will, unfortunately, SET the event.

If you've followed me so far, we've reached a situation where one thread has
the read-lock, the readers count is equal to 1 but the read_event is SET.

Now, if any thread will try to acquire a writer's lock, it will SUCCEED, and
not wait for the reader thread to release its reader-lock.

As you can see - this is a race between two readers, which allows a writer to
take the lock while a reader has it.
The comment in lines 71-72 is mesleading and isn't true.

As far as I can see, this issue was introduced 5 years ago, seen on revision
64541, and is part of all APR versions since 0.9.4.
Attached is my suggestion for a fix for this issue, as well as an addition to
the rwlock test which should cause the issue (if you're [un]lucky enough).


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org