[MacRuby] #542: Attempt to unlock a mutex which is not locked (ThreadError)

[MacRuby]

#542: Attempt to unlock a mutex which is not locked (ThreadError)
-------------------------------------+--------------------------------------
 Reporter:  valerii.hiora@…          |       Owner:  lsansonetti@…        
     Type:  defect                   |      Status:  new                  
 Priority:  major                    |   Milestone:  MacRuby 1.0          
Component:  MacRuby                  |    Keywords:                       
-------------------------------------+--------------------------------------

Comment(by jhemmelg@…):

 I took a further look through the mutex code and found what I think is
 part of the problem.  In the file thread.c, rb_mutex_unlock0(), line 1510:

 {{{
         pthread_assert(pthread_mutex_unlock(&m->mutex));
         m->thread = NULL;
 }}}

 The assignment to m->thread is made after m->mutex is unlocked.  This
 appears to create a race condition.  I swapped these two lines and it made
 my test program work.

 {{{
         m->thread = NULL;
         pthread_assert(pthread_mutex_unlock(&m->mutex));
 }}}

 With this change my test program no longer fails.  However, the error from
 running the imap test has changed:

 {{{
 $ DYLD_LIBRARY_PATH=. ./macruby -e "require 'net/imap'; imap =
 Net::IMAP.new('imap.gmail.com', 993, true);
 imap.login('somebody','somepassword')"
 /Library/Frameworks/MacRuby.framework/Versions/0.11/usr/lib/ruby/1.9.2/monitor.rb:189:in
 `mon_exit': Attempt to unlock a mutex which is locked by another thread
 (ThreadError)
         from
 /Library/Frameworks/MacRuby.framework/Versions/0.11/usr/lib/ruby/1.9.2/monitor.rb:203:in
 `synchronize'
         from -e:1:in `<main>'
 }}}

 I took a look at the code for MonitorMixin, which imap uses for the
 synchronize() call.  It looks like the error I am getting is impossible,
 because the monitor code saves and checks the thread id outside of the
 mutex doing the same thing.  The monitor code doesn't complain about the
 thread id's not matching but the mutex does.  I will look into this
 further.

 I can not be certain that there are no further race conditions in the
 mutex code.  This is why this is not submitted as a patch.

 I have a couple of questions about the mutex implementation.  I was
 looking into the code for mutex in the 1.9.2-p136 release and it looks
 fairly different.  In that code the thread variable is used as the ruby-
 level mutex variable, and the mutex is used only to protect access to it.
 Why are the implementations different and would it make sense to pull that
 implementation over?  Alternatively, would an implementation of mutex
 using the high-performance semaphore from GCD be worth considering?  Or
 would it be better to just review the existing code for race conditions
 and move on from there?  I would be happy to work on this no matter the
 direction, but I don't want to waste time working on something that would
 not be acceptable to the team.

-- 
Ticket URL: <http://www.macruby.org/trac/ticket/542#comment:7>
MacRuby <http://macruby.org/>