Issue #4765 has been reported by Motohiro KOSAKI.
Bug #4765: signal が正しくマスクされておらず main thread でシグナルハンドラが動いている
Author: Motohiro KOSAKI
Status: Assigned
Priority: Normal
Assignee: Motohiro KOSAKI
Category: core
Target version: 1.9.x
ruby -v: ruby 1.9.3dev (2011-05-21 trunk 31654) [x86_64-linux]
Bug#4027 から派生させます
sigaction によるシグナルハンドラの実行はタイマースレッドにまかせているはずなのでこれは不要だと思います。
また例外発生時に rb_trap_restore_mask を読んでいるのも、trap() で例外が発生しても trap_ensure() で
シグナルマスクを戻す処理は行なわれているので不要なような気がします。元々どういう理由で呼ばれているのか
わからなかったので自信ないですけど。
レビューした結果、rb_trap_restore_mask()がまったく不要だという意見に賛成します。
- Failure:
test_status_kill(TestProcess)
[/Users/nagachika/opt/ruby-trunk/src/ruby-trunk/test/ruby/test_process.rb:1073]:
[s.exited?, s.signaled?, s.stopped?].
<[false, true, false]> expected but was
<[true, false, false]>.
原因は rb_syswait()にあります。現状 Process.wait()中は なぜか SIGHUP, SIGQUIT, SIGINTを
SIG_IGNに設定してしまうため
この間に別スレッドが SIGQUITを送るようなテストは正しく動きません。
そもそも、signal handlerはプロセスグローバルなので、処理の途中で一時的に SIG_IGNにしてはいけません。他のスレッドが
迷惑します。pthread_sigmask()に差し替えることを検討したのですが、そもそもシグナルがタイマースレッドにしか配送されない
という設計を貫く限り、いかなる処理も必要ないという結論になりました。
まつもとさん、この処理の実装はまつもとさんのように見えます。10年以上前なので無理かもしれませんが、覚えていたら
SIG_IGNを設定していた理由を教えてください。
また、sighandler() がメインスレッドから呼ばれるのは Process.spawn を実行するとまだおきてしまいます。
process.c の before_fork/after_fork で fork の時に一時的にタイマースレッドを止めてシグナルマスクを外すためです。
こちらもどうしていいものかわからないです。
fork してそのまま動き続けるようなものはどうしようもないような気がしますが、spawn するものは
fork 後に sigprogmask で外すようにするなどでなんとかならないものでしょうか。
あーでもタイマースレッドの再起動で結局一時的に外してしまいますね……
これも、一時的にシグナルマスクを一時的に外す処理自体が不要だと思います。
また、現状、chikanagaさんの報告よりも、さらに状況は悪化しており、シグナル処理を直すと
test_signal.rb#test_kill_immediately_before_termination でテストがハングします。
これは、くだんのテストが子プロセスにSIGINT送って送出されてくる例外を確認しているのですが、
test/unit/parallel.rb#run() に Signal.trap(:INT,“IGNORE”) という極悪な文があるため、
テストでSIGINTが動かなくなる仕様変更がこっそり行われているためです。
soraさん、この行の意図はなんですか？代替案を考える必要があると思いますが、このような
子プロセスに伝搬する設定をこっそり行うのは許容できないと思います。テストに支障が出るし
第一 Ctrl-C がなかなか効かなくて test-all するときにイライラします。
ついでに以下の変更を行いました
・SIGPIPEのハンドラを空関数からSIG_IGNに変更。空関数にするぐらいならユーザ空間に
処理を戻すだけ無駄。それに、ざっと見たところrubyの中でEPIPEをハンドリングしてない場所はない
・処理の最後で、ruby_default_signal() でシグナルハンドラをSIG_DFLに戻している箇所は
シグナルマスクも解除するよう変更
添付のパッチで test-all が全て通ることを確認出来ています。
これは当面コミットせずに1.9.4に回そうと思っています。理由は
・これを直さないと致命的な状況になるような bug report はきてない
・長年放置されていたせいで、すでに複数箇所いまの挙動に依存するコードが見つかっており
安定化に時間がかかる可能性がある
と考えているため