ruby-core

SASADA Koichi <[email protected]> wrote:
> sorry for late response.
> I have no objection about this patch. thank you.
> 
> one question.
> 
> ```
> 	list_for_each_safe(&mutex->waitq, cur, next, node) {
> 	    list_del_init(&cur->node);
> 	    switch (cur->th->state) {

Oops, that should be status, not state:

	    switch (cur->th->status) {

> 	    case THREAD_KILLED:
> 		continue;
> 	    case THREAD_STOPPED:
> 	    case THREAD_RUNNABLE:
> 	    case THREAD_STOPPED_FOREVER:
> 		rb_threadptr_interrupt(cur->th);
> 		goto found;
> 	    }
> 	}
> ```
> `rb_mutex_lock()` set `th->status` as `THREAD_STOPPED_FOREVER` before
> native sleep, but the above code quoted from `rb_mutex_unlock_th()`.
> 
> What kind of situation do you assume when the thread status is other
> than `THREAD_STOPPED_FOREVER`?

Back to your original question.  THREAD_RUNNABLE is possible
if somebody uses Thread#run:

    require 'thread'
    m = Mutex.new
    th = Thread.new do
      sleep 0.1 # wait for main thread to get lock
      m.synchronize do
	sleep
      end
    end

    m.synchronize do
      sleep 0.2 # wait for th to block on m.synchronize
      th.run
    end

I am not sure about other statuses.  Maybe exit/GC can trigger
THREAD_KILLED, the mutex_free->rb_mutex_unlock_th call chain
looks like it might due to GC ordering.  Anyways, I will add
comments here when I commit.

> Thanks,
> Koichi

Thank you for the review!

Unsubscribe: <mailto:[email protected]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>