[Ruby 1.9 - Bug #4911][Open] timer thread function() が thead unsafe

Issue #4911 has been reported by Motohiro KOSAKI.


Bug #4911: timer_thread_function() が thead unsafe

Author: Motohiro KOSAKI
Status: Open
Priority: Normal
Assignee: Motohiro KOSAKI
Category: core
Target version: 1.9.x
ruby -v: trunk

どうして、これを先月気づかなかったのだろう。という罪悪感があるのですが、
target 1.9.x で起票します。

timer_thread_function()で vm->running_thread にアクセスするのは
thread unsafe な気がします

0.スレッドAがrunning_threadである
1.タイマースレッドがvm->running_threadをレジスタにのせる
(レジスタにスレッドAのアドレスがのっかる)
2.コンテキストスイッチ
3.スレッドAが終了。スレッドBがrunning_threadになる
4.スレッドAの rb_thead_t がfreeされる
5.コンテキストスイッチ
6.そんなkとはつゆしらず、タイマースレッドはスレッドAの
アドレスに対して th->interrupt_flag |= 1; するのでメモリ破壊


static void
timer_thread_function(void *arg)
{
rb_vm_t vm = GET_VM(); / TODO: fix me for Multi-VM */

/* for time slice */
RUBY_VM_SET_TIMER_INTERRUPT(vm->running_thread);      ※ここ

/* check signal */
rb_threadptr_check_signal(vm->main_thread);

}

Issue #4911 has been updated by kosaki (Motohiro KOSAKI).

Assignee changed from kosaki (Motohiro KOSAKI) to ko1 (Koichi Sasada)

I’ve made a patch.

ko1, could you please review it?


Bug #4911: timer_thread_function() が thead unsafe

Author: kosaki (Motohiro KOSAKI)
Status: Assigned
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: core
Target version: 2.0.0
ruby -v: trunk

どうして、これを先月気づかなかったのだろう。という罪悪感があるのですが、
target 1.9.x で起票します。

timer_thread_function()で vm->running_thread にアクセスするのは
thread unsafe な気がします

0.スレッドAがrunning_threadである
1.タイマースレッドがvm->running_threadをレジスタにのせる
(レジスタにスレッドAのアドレスがのっかる)
2.コンテキストスイッチ
3.スレッドAが終了。スレッドBがrunning_threadになる
4.スレッドAの rb_thead_t がfreeされる
5.コンテキストスイッチ
6.そんなkとはつゆしらず、タイマースレッドはスレッドAの
アドレスに対して th->interrupt_flag |= 1; するのでメモリ破壊


static void
timer_thread_function(void *arg)
{
rb_vm_t vm = GET_VM(); / TODO: fix me for Multi-VM */

/* for time slice */
RUBY_VM_SET_TIMER_INTERRUPT(vm->running_thread);      ※ここ

/* check signal */
rb_threadptr_check_signal(vm->main_thread);

}

Issue #4911 has been updated by ko1 (Koichi Sasada).

問題はわかったのですが,
これがどうして解決するのかわかりません.
(結局,+ if (!vm->running_thread->yielding) の部分で running_thread
を触っているように見える)

解説してもらえないでしょうか.


では,どうやって解決するかというと難しいですね.タイマスレッドが動いている間は free しない,とか,そういうのになりそうな感じ.


Bug #4911: timer_thread_function() が thead unsafe

Author: kosaki (Motohiro KOSAKI)
Status: Assigned
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: core
Target version: 2.0.0
ruby -v: trunk

どうして、これを先月気づかなかったのだろう。という罪悪感があるのですが、
target 1.9.x で起票します。

timer_thread_function()で vm->running_thread にアクセスするのは
thread unsafe な気がします

0.スレッドAがrunning_threadである
1.タイマースレッドがvm->running_threadをレジスタにのせる
(レジスタにスレッドAのアドレスがのっかる)
2.コンテキストスイッチ
3.スレッドAが終了。スレッドBがrunning_threadになる
4.スレッドAの rb_thead_t がfreeされる
5.コンテキストスイッチ
6.そんなkとはつゆしらず、タイマースレッドはスレッドAの
アドレスに対して th->interrupt_flag |= 1; するのでメモリ破壊


static void
timer_thread_function(void *arg)
{
rb_vm_t vm = GET_VM(); / TODO: fix me for Multi-VM */

/* for time slice */
RUBY_VM_SET_TIMER_INTERRUPT(vm->running_thread);      ※ここ

/* check signal */
rb_threadptr_check_signal(vm->main_thread);

}

(2012/09/21 15:10), ko1 (Koichi Sasada) wrote:

では,どうやって解決するかというと難しいですね.タイマスレッドが動いている間は free しない,とか,そういうのになりそうな感じ.

勘で書いてみたんですが,こんな感じでしょうか.再現が出来なさそうなので,
テストの書きようが無さそう.

Index: vm_core.h

— vm_core.h (revision 37007)
+++ vm_core.h (working copy)
@@ -312,6 +312,7 @@ typedef struct rb_vm_struct {
int thread_abort_on_exception;
unsigned long trace_flag;
volatile int sleeper;

  • volatile struct rb_thread_struct *timer_thread_target;

    /* object management */
    VALUE mark_object_ary;
    Index: thread.c
    ===================================================================
    — thread.c (revision 37007)
    +++ thread.c (working copy)
    @@ -3388,9 +3388,13 @@ timer_thread_function(void *arg)
    rb_vm_t vm = GET_VM(); / TODO: fix me for Multi-VM */

    /* for time slice */

  • vm->timer_thread_target = vm->running_thread;

  • {
    if (!vm->running_thread->yielding) {
    RUBY_VM_SET_TIMER_INTERRUPT(vm->running_thread);
    }

  • }

  • vm->timer_thread_target = 0;

    /* check signal */
    rb_threadptr_check_signal(vm->main_thread);
    Index: vm.c
    ===================================================================
    — vm.c (revision 37007)
    +++ vm.c (working copy)
    @@ -1724,6 +1724,9 @@ thread_free(void *ptr)
    free(th->altstack);
    }
    #endif

  •  while (GET_VM()->timer_thread_target == ptr) {
    
  • rb_thread_schedule(); /* Timer thread seeing my thread */

  •  }
     ruby_xfree(ptr);
    

    }
    if (ruby_current_thread == th)

Issue #4911 has been updated by kosaki (Motohiro KOSAKI).

問題はわかったのですが,
これがどうして解決するのかわかりません.
(結局,+ if (!vm->running_thread->yielding) の部分で running_thread を触っているように見える)
解説してもらえないでしょうか.

あ、しまった。放置しすぎていてコンテキストスイッチだけを救うパッチを書いってしまった。本当に問題なのはスイッチ元のスレッドがfreeされていてメモリ破壊に至るケースなのでおっしゃるとおり考慮不足です


Bug #4911: timer_thread_function() が thead unsafe

Author: kosaki (Motohiro KOSAKI)
Status: Assigned
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: core
Target version: 2.0.0
ruby -v: trunk

どうして、これを先月気づかなかったのだろう。という罪悪感があるのですが、
target 1.9.x で起票します。

timer_thread_function()で vm->running_thread にアクセスするのは
thread unsafe な気がします

0.スレッドAがrunning_threadである
1.タイマースレッドがvm->running_threadをレジスタにのせる
(レジスタにスレッドAのアドレスがのっかる)
2.コンテキストスイッチ
3.スレッドAが終了。スレッドBがrunning_threadになる
4.スレッドAの rb_thead_t がfreeされる
5.コンテキストスイッチ
6.そんなkとはつゆしらず、タイマースレッドはスレッドAの
アドレスに対して th->interrupt_flag |= 1; するのでメモリ破壊


static void
timer_thread_function(void *arg)
{
rb_vm_t vm = GET_VM(); / TODO: fix me for Multi-VM */

/* for time slice */
RUBY_VM_SET_TIMER_INTERRUPT(vm->running_thread);      ※ここ

/* check signal */
rb_threadptr_check_signal(vm->main_thread);

}

Issue #4911 has been updated by kosaki (Motohiro KOSAKI).

で、Comment#4のパッチだと残念なことにタイマースレッドとスレッド解放ルートに共通のロックがないため、グローバル変数に代入してもメモリバリア的な意味でthread_free()
からは可視にならなさそう

Bug #4911: timer_thread_function() が thead unsafe

Author: kosaki (Motohiro KOSAKI)
Status: Assigned
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: core
Target version: 2.0.0
ruby -v: trunk

どうして、これを先月気づかなかったのだろう。という罪悪感があるのですが、
target 1.9.x で起票します。

timer_thread_function()で vm->running_thread にアクセスするのは
thread unsafe な気がします

0.スレッドAがrunning_threadである
1.タイマースレッドがvm->running_threadをレジスタにのせる
(レジスタにスレッドAのアドレスがのっかる)
2.コンテキストスイッチ
3.スレッドAが終了。スレッドBがrunning_threadになる
4.スレッドAの rb_thead_t がfreeされる
5.コンテキストスイッチ
6.そんなkとはつゆしらず、タイマースレッドはスレッドAの
アドレスに対して th->interrupt_flag |= 1; するのでメモリ破壊


static void
timer_thread_function(void *arg)
{
rb_vm_t vm = GET_VM(); / TODO: fix me for Multi-VM */

/* for time slice */
RUBY_VM_SET_TIMER_INTERRUPT(vm->running_thread);      ※ここ

/* check signal */
rb_threadptr_check_signal(vm->main_thread);

}

(2012/09/21 17:44), kosaki (Motohiro KOSAKI) wrote:

で、Comment#4のパッチだと残念なことにタイマースレッドとスレッド解放ルートに共通のロックがないため、グローバル変数に代入してもメモリバリア的な意味でthread_free()
からは可視にならなさそう

うーん,なるほど.
真面目に mutex で囲いますかねぇ.それ以外の冴えたやり方はないかなぁ.
別に,ここでオーバヘッドになることもないだろうから,いっか.

Issue #4911 has been updated by ko1 (Koichi Sasada).

Assignee changed from ko1 (Koichi Sasada) to kosaki (Motohiro KOSAKI)

これは忘れてはいけなかった気がする.
小崎先生お願いします.

Bug #4911: timer_thread_function() が thead unsafe

Author: kosaki (Motohiro KOSAKI)
Status: Assigned
Priority: Normal
Assignee: kosaki (Motohiro KOSAKI)
Category: core
Target version: 2.0.0
ruby -v: trunk

どうして、これを先月気づかなかったのだろう。という罪悪感があるのですが、
target 1.9.x で起票します。

timer_thread_function()で vm->running_thread にアクセスするのは
thread unsafe な気がします

0.スレッドAがrunning_threadである
1.タイマースレッドがvm->running_threadをレジスタにのせる
(レジスタにスレッドAのアドレスがのっかる)
2.コンテキストスイッチ
3.スレッドAが終了。スレッドBがrunning_threadになる
4.スレッドAの rb_thead_t がfreeされる
5.コンテキストスイッチ
6.そんなkとはつゆしらず、タイマースレッドはスレッドAの
アドレスに対して th->interrupt_flag |= 1; するのでメモリ破壊


static void
timer_thread_function(void *arg)
{
rb_vm_t vm = GET_VM(); / TODO: fix me for Multi-VM */

/* for time slice */
RUBY_VM_SET_TIMER_INTERRUPT(vm->running_thread);      ※ここ

/* check signal */
rb_threadptr_check_signal(vm->main_thread);

}