Backport #2603: NetBSD 5.0以降でpthreadの処理に由来する不具合

こんばんは。

In message <[e-mail]>
	on Thu, 14 Jan 2010 20:03:41 +0900,
	[e-mail] (MIURA, Fumiaki) wrote:
> NTTレゾナントの三浦です。不完全なpatchを書いてすみません。
あのパッチが、調べるとっかかりになりました。たいへん、助かってますので
謝らないでください。(感謝、感激に近い状態で、ほんとうにありがとうござい
ます。)

>> [ruby-dev:40051]にて対策が提案されていますが、これを適用しても上記の状況は
>> 解消されないため、先に挙げた問題は発生しなくなるケースもありましたが、完全
>> には解決しませんでした。
あ、この辺りの書き方にトゲがあるように読み取れますな、すんません。

>> patch-1
>>     子プロセスの方で pthread関係の関数の呼び出しを止めるようにしたもの
> 
> 「#if defined(__NetBSD__)」でも(rb_thread_start_timer()経由)で
> pthread_cond_wait()を呼び出してるみたいですが、こいつも追い出すべきで
> はないですか?
> 勘違い?
取り敢えず、

1. 2回目のバッククォートの展開での子プロセス
2. Net::HTTPを使っただけのつもりのプロセス

で、thread_timer()から脱出できなくなる問題が解決するところまで確認した
状況で、全部のパターンを追えたというわけではありません。つまり、

> それと、fork()して作られた子供のrubyはruby threadを作れない、あるいは
> 作れても動作は異なるということでしょうか? 
といったケースはテストも何もしていませんねぇ。

-- 
神戸 隆博 / Takahiro Kambe 

P.S.
1.9とかならともかく、1.8.7でまっとうに動かなくなったのは、かなりのショッ
クでした。

こんにちは。

In message <[e-mail]>
	on Fri, 15 Jan 2010 09:26:02 +0900,
	KOSAKI Motohiro <[e-mail]> wrote:
>> で、thread_timer()から脱出できなくなる問題が解決するところまで確認した
>> 状況で、全部のパターンを追えたというわけではありません。つまり、
>> 
>> > それと、fork()して作られた子供のrubyはruby threadを作れない、あるいは
>> > 作れても動作は異なるということでしょうか? 
>> といったケースはテストも何もしていませんねぇ。
> 
> すいません、よくわかっていないので、本件の方針を確認させてもらえませんか？
私が理解している範囲で、想定できるケースは以下の3つではないかと思います。

1. fork(2)した子プロセスがexec(3)する場合
2. プロセス自身がfork(2)せずにexec(3)する場合
3. fork(2)した子プロセスがexec(3)せずに動作を続ける場合

今回は1.と2.について取り上げ、redmineに登録したpatch-1ではとにかく1.だ
けを、patch2では2.のケースも考慮した修正を行ったつもりです。

1.については、fork(2)した子プロセスがpthread_*()な関数の呼び出しを避け
るようにしたつもりです。

2.については、fork(2)した子プロセスがpthread_*()な関数の呼び出しを避け
つつ、fork(2)せずにexec(3)する場合は稼働しているpthreadの後始末を行う
ようにしたつもりです。

3.についてはテストすら行っていません。(test caseがないとも言えます。)
ただ、C言語等でpthreadを使用したプログラミングでも、fork(2)と混ぜるこ
とは本質的な危険性を伴うため、どこまで3.のようなケースが必要なのか若干
の疑問を感じるところもあります。

というわけで、3.はともかく1.や2.は修正しないとまずいだろうというのが
私の要望です。


なお、patch-1とpatch-2のいずれかを適用しないと、fork(2)すらしないケース
でも、timer_threadが待ち続ける状況も発生することに後から気付きました。

>> (1) fork(2)した子プロセスは非同期シグナルに対して安全な関数だけを使用でき、
>>     pthread_*()な関数も使用できません(非同期シグナルに対して安全でない)。
> 
> という所なんですが、そもそもmalloc()を呼ばないことを保証できないような
> 構造にすでになっているので、規格的な観点で絶対safeといえる実装に変更する事は
> 困難と予想しています。
次元が違いますが、GNU iconvを前提とした //translit を使ったプログラム
と同様な状態にRuby自体がなっているとも考えられます。

長期的には、pthreadに対して安全な構造にした方が良いと思いますが、短期
的には難しいでしょう。

> NetBSDの仕様（何をしたら刺さるか）を確認し、そこだけを避けるように変更する。
> という方針だと思ってよいのでしょうか？
「NetBSDの仕様」ではなく、あくまでもfork(2)/pthread(3)の仕様を守るよう
にして欲しいというのが「要望」です。

とは言うものの、現実的なところとして、

o 明らかにpthread(3)関係の関数呼び出しはまずい。
o 間接的にpthread関係の関数呼び出しもまずい。
o 非同期シグナルに対して安全でない関数でも、pthreadのデータ構造等に
  直接関与しなければ案外大丈夫かもしれない。

といった気がします。

-- 
神戸 隆博(かんべ たかひろ)		at 仕事場 

まつもと ゆきひろです

In message "Re: [ruby-dev:40092] Re: [Bug #2603] NetBSD 5.0以降でpthreadの処理に由来する不具合"
    on Fri, 15 Jan 2010 12:25:52 +0900, Takahiro Kambe <[e-mail]> writes:

|私が理解している範囲で、想定できるケースは以下の3つではないかと思います。
|
|1. fork(2)した子プロセスがexec(3)する場合
|2. プロセス自身がfork(2)せずにexec(3)する場合
|3. fork(2)した子プロセスがexec(3)せずに動作を続ける場合
|
|今回は1.と2.について取り上げ、redmineに登録したpatch-1ではとにかく1.だ
|けを、patch2では2.のケースも考慮した修正を行ったつもりです。

patch-2を当てることを提案します。

In message <[e-mail]>
	on Fri, 15 Jan 2010 12:32:27 +0900,
	Yukihiro Matsumoto <[e-mail]> wrote:
> |私が理解している範囲で、想定できるケースは以下の3つではないかと思います。
> |
> |1. fork(2)した子プロセスがexec(3)する場合
> |2. プロセス自身がfork(2)せずにexec(3)する場合
> |3. fork(2)した子プロセスがexec(3)せずに動作を続ける場合
> |
> |今回は1.と2.について取り上げ、redmineに登録したpatch-1ではとにかく1.だ
> |けを、patch2では2.のケースも考慮した修正を行ったつもりです。
> 
> patch-2を当てることを提案します。
#if defined(__NetBSD__) とかしていますが、もう少し気の利いた条件名を
考えて、該当するOSではそれを定義するとかしたかったのですが、そこまで
手が回りませんでした。

-- 
神戸 隆博(かんべ たかひろ)		at 仕事場 

まつもと ゆきひろです

In message "Re: [ruby-dev:40094] Re: [Bug #2603] NetBSD 5.0以降でpthreadの処理に由来する不具合"
    on Fri, 15 Jan 2010 12:34:57 +0900, Takahiro Kambe <[e-mail]> writes:

|#if defined(__NetBSD__) とかしていますが、もう少し気の利いた条件名を
|考えて、該当するOSではそれを定義するとかしたかったのですが、そこまで
|手が回りませんでした。

それはパッチを当ててからおいおい考えましょう。

このpatch-2、公開 API である rb_thread_stop_timer の引数を void から int に変えているんですが、大丈夫ですか。

また、ruby_1_8 や trunk は試されてるでしょうか。
trunk は先送る (& 一部事情が異なる) としても 1.8.7 に入れる前に ruby_1_8 に入れてそれを backport という形になると思うのです。

In message <[e-mail]>
	on Fri, 15 Jan 2010 12:59:26 +0900,
	Yui NARUSE <[e-mail]> wrote:
> また、ruby_1_8 や trunk は試されてるでしょうか。
手元の1.8.7だけで試しています。

FYIとしてですが、1.9.1 patchlevel 378では同様の問題は確認できていません。
(単に表面化していない可能性はあります。)

> trunk は先送る (& 一部事情が異なる) としても 1.8.7 に入れる前に
> ruby_1_8 に入れてそれを backport という形になると思うのです。
私はコミットできるわけではないので、後はお願いします。

-- 
神戸 隆博(かんべ たかひろ)		at 仕事場 

• Status changed from Open to Assigned
• Assignee set to knu (Akinori MUSHA)

とりあえず1.8.8devでも試してみていただくってのは難しい話ですか?

まあNetBSDは1.8.7と心中しますと言いきっていただけるなら、1.8.7にじかに当てるのにやぶさかではありませんが。

In message <[e-mail]>
	on Fri, 15 Jan 2010 17:23:52 +0900,
	KOSAKI Motohiro <[e-mail]> wrote:
>> というわけで、3.はともかく1.や2.は修正しないとまずいだろうというのが
>> 私の要望です。
> 
> 三浦さんと同じく2の必要性に疑問が・・・
> （根拠も同じです）
execするんだから要らないような気はしてたのですが、既存のコードが律儀に
行っているように見えたことと、exec()失敗した後に続ける場合を念頭に置い
てました。

田中さんによると、Mac OSのように必要な環境もあるということのようですね。

>> o 明らかにpthread(3)関係の関数呼び出しはまずい。
>> o 間接的にpthread関係の関数呼び出しもまずい。
>> o 非同期シグナルに対して安全でない関数でも、pthreadのデータ構造等に
>>   直接関与しなければ案外大丈夫かもしれない。
> 
> ええと、一般論の話をするならlibcレベルでもreentrantじゃないけどスレッ
> ドセーフな関数は
> わりと一杯あって、それは要するに内部でpthread_mutexかそれに類似したロックを
> 取っているということで、誰かがそういうロックを取っている最中に
> forkすると、以後libcのスレッドセーフな関数を呼ぶと刺さるという結果に
> なる可能性があります。
確かにそうです。

> NetBSDの話をしているなら、きっと神戸さんが正しいと思います
安直に信じてはダメです。:-)

-- 
神戸 隆博 / Takahiro Kambe 

On NetBSD 5.0.1 and Ruby 1.8.8dev, this problem doesn't happen:

% make TESTS='-v -xopenssl' test-all|tee t.log

  1) Failure:
test_break(TestAssignment) [/home/naruse/ruby_1_8/test/ruby/test_assignment.rb:291]:
<[]> expected but was
<nil>.

  2) Failure:
test_next(TestAssignment)
    [/home/naruse/ruby_1_8/test/ruby/test_assignment.rb:364:in `r'
     /home/naruse/ruby_1_8/test/ruby/test_assignment.rb:372:in `test_next']:
<[]> expected but was
<nil>.

  3) Failure:
test_return(TestAssignment) [/home/naruse/ruby_1_8/test/ruby/test_assignment.rb:158]:
<[]> expected but was
<nil>.

  4) Failure:
test_yield(TestAssignment)
    [/home/naruse/ruby_1_8/test/ruby/test_assignment.rb:95:in `test_yield'
     /home/naruse/ruby_1_8/test/ruby/test_assignment.rb:95:in `f'
     /home/naruse/ruby_1_8/test/ruby/test_assignment.rb:95:in `test_yield']:
<[]> expected but was
<nil>.

  5) Failure:
test_uninitialized(TestThreadGroup) [/home/naruse/ruby_1_8/test/ruby/test_thread.rb:421]:
<ThreadError> exception expected but was
Class: <NoMethodError>
Message: <"undefined method `start' for #<#<Class:0xb93febd4>:0xb93fe864 run>">
---Backtrace---
/home/naruse/ruby_1_8/test/ruby/test_thread.rb:421:in `test_uninitialized'
/home/naruse/ruby_1_8/test/ruby/test_thread.rb:421:in `test_uninitialized'
---------------

2362 tests, 1346564 assertions, 5 failures, 0 errors
*** Error code 1

Stop.
make: stopped in /home/naruse/ruby_1_8

念のためというか、最初に書いていなければいけなかったのですが、
configure時に --enable-pthread はされてましたでしょうか?
(手元で素のRubyをビルドして再現しなくて、pkgsrcでは --enable-pthread してる
ことを思い出したのでした。)

In message <[e-mail]>
	on Mon, 18 Jan 2010 13:49:18 +0900,
	[e-mail] (MIURA, Fumiaki) wrote:
> At Thu, 14 Jan 2010 12:32:05 +0900, Takahiro Kambe wrote:
>> Ruby 1.8.7 patchlevel 249はNetBSD 5.0以降では、pthreadの処理に由来する問題に
>> より、そのままでは実用に絶えません。
補足すると、--enable-pthread で configureした場合です。

>> * Rubyでthreadを使用したプログラム
>> * systemやバッククォートで子プロセスを呼び出したプログラム
>> 
>> これらではRuby内部のpthreadによるthread_timer内で待ち続ける状態になって、
>> プロセスが SIGKILL でないと殺せない状態に陥る場合があります。
> 
> というのは具体的にどこで起こるのでしょう?
thread_timer() (in eval.c)です。

以上、取り敢えず。

-- 
神戸 隆博(かんべ たかひろ)		at 仕事場 

成瀬です。

(2010/01/18 14:05), Takahiro Kambe wrote:
> 念のためというか、最初に書いていなければいけなかったのですが、
> configure時に --enable-pthread はされてましたでしょうか?
> (手元で素のRubyをビルドして再現しなくて、pkgsrcでは --enable-pthread してる
> ことを思い出したのでした。)

あー、完全に存在を忘れていました、ですよね当然必要ですよね。
で、指定すると再現しました、なるほど。

-- 
NARUSE, Yui  <[e-mail]>

こんにちは。

In message <[e-mail]>
	on Mon, 18 Jan 2010 11:40:44 +0900,
	[e-mail] (MIURA, Fumiaki) wrote:
> 改良版かもしれないfork patchその2を作ってみました。
取り敢えず、

 * RUBY_1_8 on NetBSD 5.0_STABLE
 * RUBY_1_8_7 on NetBSD current (4.99.23)

でテストすると、同じ残念な結果となりました。前者の方の結果です。

o 環境

% uname -mrs
NetBSD 5.0_STABLE i386
% ./ruby -v
ruby 1.8.8dev (2010-01-17 revision 26335) [i386-netbsdelf5.0.]

o 構成

% sh configure --enable-pthread

	(共有ライブラリを使用していません。)

o コンパイル

% make

	(make installは行っていません。)

o 状況

./ruby -rpurelib.rb ./test/runner.rb --basedir=./test --runner=console -v thread 
と実行しているプロセスが以下のsafe_mutex_lock()、CPPを展開すると
pthread_mutex_lock()でブロックしていました。gdbで捕まえたstack traceは、

#0  0xbbbe2960 in pthread_mutex_lock () from /usr/lib/libpthread.so.0
#1  0x08056b7d in thread_timer (dummy=0xbfbfe2fc) at eval.c:12491
#2  0xbbbe59b7 in pthread_create () from /usr/lib/libpthread.so.0
#3  0xbbb0c3d0 in swapcontext () from /usr/lib/libc.so.12

となり、#1付近のソースコードは以下のようになっています。

12486       sigset_t all_signals;
12487   
12488       sigfillset(&all_signals);
12489       pthread_sigmask(SIG_BLOCK, &all_signals, 0);
12490   
12491       safe_mutex_lock(&running->lock);
12492       pthread_cond_signal(start);
12493   
12494   #define WAIT_FOR_10MS() \
12495       pthread_cond_timedwait(&running->cond, &running->lock, get_ts(&to, PER_NANO/100))

何も変更していない場合は、このWAIT_FOR_10MS()を定義している後に続くルー
プ内から抜け出れないで回っている状態でしたが、今度は完全に止まっている
ようで、ktruss -p とかしても何もトレースした出力はされない状態です。

-- 
神戸 隆博(かんべ たかひろ)		at 仕事場 

そういえば、現状ミニマムな再現ケースって出ていませんよね?
参考までに、
* ./ruby -e'Thread.new{sleep};`./ruby -e"1"`'
* ./ruby -e'Thread.new{sleep};fork{}'

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

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

• Status changed from Closed to Assigned
• Assignee changed from knu (Akinori MUSHA) to shyouhei (Shyouhei Urabe)

Urabeさま

調査の結果、bug#2279は本件と同じ原因であることが分かりました。
bug#2279に本パッチの1.8.7へのバックポートパッチと、ifdef netbsdをifdef netbsd or linuxに変更するパッチを添付しておいたので、一度見ていただけないでしょうか。

間違えた bug#2739 です。全然あってないぞ ← じぶん

• Status changed from Assigned to Closed

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