Invalid read may occur when raising an exception to a thread that waits for mutex

e$B1sF#$G$9!#e(B

e$B0J2<$N$h$&$K$9$k$H%O%s%0$9$k$3$H$,$"$j$^$9!#e(B

$ ./ruby -ve ’
N = 10

m = [Mutex.new]
m.first.lock

t = Thread.new do
begin
m.first.lock # (1)
rescue
loop { } # (3)
end
end

sleep 1
t.raise # (2)

m.clear
sleep 1
GC.start # (4)

t.raise # (5)

ruby 1.9.0 (2008-05-28 revision 16675) [i686-linux]
^Z

e$BN.$l$O$3$s$J46$8$G$9!#e(B

  • (1) e$B$G%9%l%C%Ie(B t e$B$Ne(B unblock_function e$B$,e(B
    lock_interrupt e$B$Ke(B
    e$B$J$j!"e(Bmutex e$B$N2rJ|BT$A$KF~$k!#e(B

  • (2) e$B$G%9%l%C%Ie(B t e$B$Ne(B unblock_function (= lock_interrupt)
    e$B$,e(B
    e$B8F$P$l$k!#e(B

  • e$B%9%l%C%Ie(B t e$B$Ne(B unblock_function
    e$B$O%/%j%"$5$l$k$Y$-$@$,!“e(B
    e$B%/%j%”$5$l$J$$$^$^%9%l%C%Ie(B t e$B$N<B9T$Oe(B (3) e$B$K0$k!#e(B

  • (4) e$B$Ge(B mutex e$B$re(B free e$B$9$ke(B (e$B$?$@$74D6-0MB8e(B)
    e$B!#e(B

  • (5) e$B$G%9%l%C%Ie(B t e$B$Ne(B unblock_function (= lock_interrupt)
    e$B$,e(B
    e$B8F$P$l$k$,!"e(Block_interrupt e$B$NCf$G2rJ|:Q$_$Ne(B mutex
    e$B$K?($k!#e(B

e$B<B:]!"e(Bvalgrind e$B$K$+$1$k$H!"e(Block_interrupt e$BFb$Ge(B Invalid
read
e$B$7$F$$$k$N$,$o$+$j$^$9!#e(B

==15890== Invalid read of size 4
==15890== at 0x811AA87: lock_interrupt (thread.c:2439)
==15890== by 0x81181D9: rb_thread_interrupt (thread.c:222)
==15890== by 0x8119284: rb_thread_ready (thread.c:864)
==15890== by 0x81192EE: rb_thread_raise (thread.c:884)
==15890== by 0x811941F: thread_raise_m (thread.c:960)
==15890== by 0x810A59B: call_cfunc (vm_insnhelper.c:286)
==15890== by 0x8114F32: vm_call_cfunc (vm_insnhelper.c:376)
==15890== by 0x8114A7C: vm_call_method (vm_insnhelper.c:508)
==15890== by 0x8110F8B: vm_eval (insns.def:1048)
==15890== by 0x8115135: vm_eval_body (vm.c:1022)
==15890== by 0x811575C: rb_iseq_eval (vm.c:1226)
==15890== by 0x805A0FB: ruby_exec_node (eval.c:221)
==15890== Address 0x44B6F04 is 76 bytes inside a block of size 80
free’d
==15890== at 0x401CFA5: free (vg_replace_malloc.c:233)
==15890== by 0x805EA4E: ruby_xfree (gc.c:425)
==15890== by 0x811A899: mutex_free (thread.c:2340)
==15890== by 0x80606C4: obj_free (gc.c:1498)
==15890== by 0x8060214: gc_sweep (gc.c:1371)
==15890== by 0x8060B0C: garbage_collect (gc.c:1704)
==15890== by 0x80614AF: rb_gc (gc.c:2119)
==15890== by 0x8060B6B: rb_gc_start (gc.c:1751)
==15890== by 0x810A5B1: call_cfunc (vm_insnhelper.c:289)
==15890== by 0x8114F32: vm_call_cfunc (vm_insnhelper.c:376)
==15890== by 0x8114A7C: vm_call_method (vm_insnhelper.c:508)
==15890== by 0x8110F8B: vm_eval (insns.def:1048)

unblock_function e$B$,%/%j%"$5$l$J$$$N$Oe(B set_unblock_function
e$BFb$Ge(B
RUBY_VM_CHECK_INTS e$B$r$7$F$$$F!"$=$3$+$ie(B longjmp
e$B$9$k$;$$$G$9!#e(B
e$B$3$Ne(B RUBY_VM_CHECK_INTS e$B$OI,MW$J$s$G$7$g$&$+!#e(B

Index: thread.c

— thread.c (revision 16676)
+++ thread.c (working copy)
@@ -197,19 +197,11 @@
set_unblock_function(rb_thread_t *th, rb_unblock_function_t *func, void
*arg,
rb_unblock_function_t **oldfunc, void **oldarg)
{

  • check_ints:
  • RUBY_VM_CHECK_INTS(); /* check signal or so */
    native_mutex_lock(&th->interrupt_lock);
  • if (th->interrupt_flag) {
  • native_mutex_unlock(&th->interrupt_lock);
  • goto check_ints;
  • }
  • else {
  • if (oldfunc) *oldfunc = th->unblock_function;
  • if (oldarg) *oldarg = th->unblock_function_arg;
  • th->unblock_function = func;
  • th->unblock_function_arg = arg;
  • }
  • if (oldfunc) *oldfunc = th->unblock_function;
  • if (oldarg) *oldarg = th->unblock_function_arg;
  • th->unblock_function = func;
  • th->unblock_function_arg = arg;
    native_mutex_unlock(&th->interrupt_lock);
    }

e$B$J$+$@$G$9!#e(B

At Wed, 28 May 2008 21:42:48 +0900,
Yusuke ENDOH wrote in [ruby-dev:34874]:

unblock_function e$B$,%/%j%"$5$l$J$$$N$Oe(B set_unblock_function e$BFb$Ge(B
RUBY_VM_CHECK_INTS e$B$r$7$F$$$F!"$=$3$+$ie(B longjmp e$B$9$k$;$$$G$9!#e(B
e$B$3$Ne(B RUBY_VM_CHECK_INTS e$B$OI,MW$J$s$G$7$g$&$+!#e(B

BLOCKING_REGIONe$B$KF~$k$H$-$K$Oe(Bchecke$B$7$F$bLdBj$J$$$s$8$c$J$$$G$7$ge(B
e$B$&$+!#LdBj$,5/$-$k$N$OH4$1$k$H$-$G$9$h$M!#e(B

Index: vm_core.h

— vm_core.h (revision 16676)
+++ vm_core.h (working copy)
@@ -374,4 +374,9 @@ struct rb_vm_trap_tag {
#define USE_VALUE_CACHE 0

+struct rb_unblock_callback {

  • rb_unblock_function_t *func;
  • void *arg;
    +};

typedef struct rb_thread_struct rb_thread_t;

@@ -419,7 +424,6 @@ struct rb_thread_struct

 int interrupt_flag;
  • rb_unblock_function_t *unblock_function;
  • void *unblock_function_arg;
    rb_thread_lock_t interrupt_lock;
  • struct rb_unblock_callback unblock;

    struct rb_vm_tag *tag;
    Index: thread.c
    ===================================================================
    — thread.c (revision 16676)
    +++ thread.c (working copy)
    @@ -80,6 +80,7 @@ st_delete_wrap(st_table *table, st_data_
    #define THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION

-static void set_unblock_function(rb_thread_t *th, rb_unblock_function_t
*func, void *ptr,

  •     rb_unblock_function_t **oldfunc, void **oldptr);
    

+static void set_unblock_function(rb_thread_t *th, rb_unblock_function_t
*func, void *arg,

  •     struct rb_unblock_callback *old);
    

+static void reset_unblock_function(rb_thread_t *th, const struct
rb_unblock_callback *old);

#define GVL_UNLOCK_BEGIN() do {
@@ -96,7 +97,6 @@ static void set_unblock_function(rb_thre
rb_thread_t *__th = GET_THREAD();
int __prev_status = __th->status; \

  • rb_unblock_function_t *__oldubf; \
  • void *__oldubfarg; \
  • set_unblock_function(__th, ubf, ubfarg, &__oldubf, &__oldubfarg); \
  • struct rb_unblock_callback __oldubf; \
  • set_unblock_function(__th, ubf, ubfarg, &__oldubf);
    __th->status = THREAD_STOPPED;
    thread_debug(“enter blocking region (%p)\n”, __th);
    @@ -107,5 +107,5 @@ static void set_unblock_function(rb_thre
    thread_debug(“leave blocking region (%p)\n”, __th);
    remove_signal_thread_list(__th); \
  • set_unblock_function(__th, __oldubf, __oldubfarg, 0, 0); \
  • reset_unblock_function(__th, &__oldubf);
    if (__th->status == THREAD_STOPPED) {
    __th->status = __prev_status;
    @@ -196,5 +196,5 @@ rb_thread_debug(const char *fmt, …)
    static void
    set_unblock_function(rb_thread_t *th, rb_unblock_function_t *func, void
    *arg,
  •     rb_unblock_function_t **oldfunc, void **oldarg)
    
  •     struct rb_unblock_callback *old)
    

{
check_ints:
@@ -206,8 +206,7 @@ set_unblock_function(rb_thread_t *th, rb
}
else {

  • if (oldfunc) *oldfunc = th->unblock_function;
  • if (oldarg) *oldarg = th->unblock_function_arg;
  • th->unblock_function = func;
  • th->unblock_function_arg = arg;
  • if (old) *old = th->unblock;
  • th->unblock.func = func;
  • th->unblock.arg = arg;
    }
    native_mutex_unlock(&th->interrupt_lock);
    @@ -215,10 +214,18 @@ set_unblock_function(rb_thread_t *th, rb

static void
+reset_unblock_function(rb_thread_t *th, const struct
rb_unblock_callback *old)
+{

  • native_mutex_lock(&th->interrupt_lock);
  • th->unblock = *old;
  • native_mutex_unlock(&th->interrupt_lock);
    +}

+static void
rb_thread_interrupt(rb_thread_t *th)
{
native_mutex_lock(&th->interrupt_lock);
RUBY_VM_SET_INTERRUPT(th);

  • if (th->unblock_function) {
  • (th->unblock_function)(th->unblock_function_arg);
  • if (th->unblock.func) {
  • (th->unblock.func)(th->unblock.arg);
    }
    else {
    Index: thread_pthread.c
    ===================================================================
    — thread_pthread.c (revision 16676)
    +++ thread_pthread.c (working copy)
    @@ -425,6 +425,6 @@ native_sleep(rb_thread_t *th, struct tim
    {
    pthread_mutex_lock(&th->interrupt_lock);
  • th->unblock_function = ubf_pthread_cond_signal;
  • th->unblock_function_arg = th;
  • th->unblock.func = ubf_pthread_cond_signal;

  • th->unblock.arg = th;

    if (RUBY_VM_INTERRUPTED(th)) {
    @@ -452,6 +452,6 @@ native_sleep(rb_thread_t *th, struct tim
    }
    }

  • th->unblock_function = 0;
  • th->unblock_function_arg = 0;
  • th->unblock.func = 0;

  • th->unblock.arg = 0;

    pthread_mutex_unlock(&th->interrupt_lock);
    Index: thread_win32.c
    ===================================================================
    — thread_win32.c (revision 16676)
    +++ thread_win32.c (working copy)
    @@ -221,6 +221,6 @@ native_sleep(rb_thread_t *th, struct tim

    th->status = THREAD_STOPPED;

  • th->unblock_function = ubf_handle;
  • th->unblock_function_arg = th;
  • th->unblock.func = ubf_handle;

  • th->unblock.arg = th;

    if (RUBY_VM_INTERRUPTED(th)) {
    @@ -233,6 +233,6 @@ native_sleep(rb_thread_t *th, struct tim
    }

  • th->unblock_function = 0;
  • th->unblock_function_arg = 0;
  • th->unblock.func = 0;
  • th->unblock.arg = 0;
    th->status = status;
    }

e$B1sF#$G$9!#e(B

08/05/29 e$B$Ke(B Nobuyoshi N.[email protected]
e$B$5$s$O=q$-$^$7$?e(B:

unblock_function e$B$,%/%j%“$5$l$J$$$N$Oe(B set_unblock_function e$BFb$Ge(B
RUBY_VM_CHECK_INTS e$B$r$7$F$$$F!”$=$3$+$ie(B longjmp e$B$9$k$;$$$G$9!#e(B
e$B$3$Ne(B RUBY_VM_CHECK_INTS e$B$OI,MW$J$s$G$7$g$&$+!#e(B

BLOCKING_REGIONe$B$KF~$k$H$-$K$Oe(Bchecke$B$7$F$bLdBj$J$$$s$8$c$J$$$G$7$ge(B
e$B$&$+!#LdBj$,5/$-$k$N$OH4$1$k$H$-$G$9$h$M!#e(B

e$B$=$&$G$9$M!#$=$C$A$NJ}$,$h$5$=$&$G$9!#e(B
e$B$h$m$7$/$*4j$$$7$^$9!#e(B