Bug #4988: ObjectSpace.#define_finalizer内でMutexをロックして解放しないまま抜けるとabortする

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 1.9.3dev (2011-07-07 trunk 32437) [x86_64-linux]

[Glass_saga (Masaki Matsushita)]

=begin
次のコードを実行すると、

ObjectSpace.define_finalizer("") do
Mutex.new.lock
end

以下のようにabortします。

[BUG] thread_free: keeping_mutexes must be NULL (0x1fa0510:0x21f8ac0)
ruby 1.9.3dev (2011-07-07 trunk 32437) [x86_64-linux]

-- Control frame information -----------------------------------------------

-- C level backtrace information -------------------------------------------
../ruby_trunk/bin/ruby() [0x529687] vm_dump.c:796
../ruby_trunk/bin/ruby() [0x576768] error.c:258
../ruby_trunk/bin/ruby(rb_bug+0xa2) [0x577bc2] error.c:270
../ruby_trunk/bin/ruby(ruby_vm_destruct+0x17f) [0x52741f] vm.c:1744
../ruby_trunk/bin/ruby(ruby_cleanup+0x283) [0x4174c3] eval.c:184
../ruby_trunk/bin/ruby(ruby_run_node+0x3d) [0x4176cd] eval.c:241
../ruby_trunk/bin/ruby() [0x414849] main.c:38
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xff) [0x7f7785469eff]
../ruby_trunk/bin/ruby() [0x414739]

また、ruby 1.9.2p180ではabortせず、SEGVしました。

インタプリタの終了処理の際、eval.cのruby_cleanup()ではthread.cのrb_thread_terminate_all()が呼ばれ、そこからさらにrb_mutex_unlock_all()が呼ばれて全てのMutexが解放された後に
ruby_finalize_1()が呼ばれてrb_gc_call_finalizer_at_exit()でfinalizerが実行されますが、finalizer内で新たにMutexをロックして解放しないまま抜けてしまうと、
その後呼ばれるruby_vm_destruct()内のthread_free()で落ちます。

rb_gc_call_finalizer_at_exit()をrb_thread_terminate_all()よりも先に呼ぶようにしたところabortしなくなったので、パッチを添付します。

=end

[Glass_saga (Masaki Matsushita)]

書いてしまってから気づきましたが、rb_thread_terminate_all()の前にfinalizerを呼ぶのは明らかに問題がありますね。
無知丸出しだなあ、という感じです。
finalizerを動かした後に、もしロックされたMutexがあればもう一度rb_mutex_unlock_all()すれば良いかと思いましたが、rb_mutex_unlock_all()はthread.cからしか呼べないのでした。

[shyouhei (Shyouhei Urabe)]

at_exitの中からvm->main_threadが見えないようにするためにわざとruby_vm_run_at_exit_hooks()を今の位置で呼んでるんでしたっけ。
だとしたらこのパッチではダメですが。

Index: thread.c
===================================================================
--- thread.c    (revision 32440)
+++ thread.c    (working copy)
@@ -334,6 +334,14 @@
 static void rb_mutex_unlock_all(rb_mutex_t *mutex, rb_thread_t *th);
 static void rb_mutex_abandon_all(rb_mutex_t *mutexes);

+static void
+mutex_unlock_all_i(rb_vm_t *vm)
+{
+    rb_thread_t *th = vm->main_thread;
+    if (th)
+        rb_mutex_unlock_all(th->keeping_mutexes, th);
+}
+
 void
 rb_thread_terminate_all(void)
 {
@@ -363,6 +371,7 @@
        }
        POP_TAG();
     }
+    ruby_vm_at_exit(mutex_unlock_all_i);
 }

 static void
Index: vm.c
===================================================================
--- vm.c        (revision 32440)
+++ vm.c        (working copy)
@@ -1567,6 +1567,7 @@
        struct rb_objspace *objspace = vm->objspace;
 #endif 
        rb_gc_force_recycle(vm->self);
+       ruby_vm_run_at_exit_hooks(vm);
        vm->main_thread = 0;
        if (th) {
            thread_free(th);
@@ -1580,7 +1581,6 @@
            rb_objspace_free(objspace);
        }
 #endif
-       ruby_vm_run_at_exit_hooks(vm);
        rb_vm_gvl_destroy(vm);
        ruby_xfree(vm);
        ruby_current_vm = 0;

[kosaki (Motohiro KOSAKI)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r32446.
Masaki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• thread.c (thread_unlock_all_locking_mutexes): rename to
  rb_threadptr_unlock_all_locking_mutexes and remove static.

• vm_core.h: add rb_threadptr_unlock_all_locking_mutexes declaration.

• thread.c (thread_start_func_2): adjust the above rename.

• eval.c (ruby_cleanup): call rb_threadptr_unlock_all_locking_mutexes
  again after finalizer. [Bug #4988] [ruby-dev:44049]