matsumotory / ngx_mruby

ngx_mruby - A Fast and Memory-Efficient Web Server Extension Mechanism Using Scripting Language mruby for nginx
https://ngx.mruby.org/
988 stars 112 forks source link

Updating Fiber #517

Closed dearblue closed 11 months ago

dearblue commented 1 year ago

間が空いてしまいすいません。 この PR によって https://github.com/mruby/mruby/pull/6063 を閉じられるのではないかと思いますが、いかがでしょうか?

それと気付いたことがあったので、ひとつだけ補足をさせてください。 Nginx.redirect によって mrb_fiber_resume() が入れ子状に呼び出されることがあります。僕はこれまで問題がありそうだと考えていました。 しかしながら再考したところ Fiber#transfer が絡まなければ、コールスタックを圧迫すること以外は問題なさそうだと思うようになりました。 このことは mruby の mrb_fiber_resume() に関するドキュメント を更新する必要があります。 ただし、ちゃんとした検証を行ってからとなるので、mruby への提案は少し先になりそうです。

@matsumotory Please review.

Pull-Request Check List

dearblue commented 1 year ago

:scream_cat: もう一つ補足がありました。 現状ではテストが失敗する可能性があります。

build_config.rbconf.enable_debugconf.defines << "MRB_GC_STRESS" を追加することで再現できるはずです。

% sh test.sh

  ...略...

............................................XX....................................................
ArgumentError: ngx_mruby - rack base [location /rack_base_env] => string contains null byte
ArgumentError: ngx_mruby - rack base [method POST, location /rack_base_env] => string contains null byte
Total: 98
   OK: 96
   KO: 0
Crash: 2
 Time: 19.5605 seconds

これについては、mruby-json にパッチを当てることで成功するようになります。

https://github.com/mattn/mruby-json/pull/49

matsumotory commented 1 year ago

ありがとうございます!mruby-jsonのPRがマージされ次第こちらでも確認します。

dearblue commented 1 year ago

mruby 本体に C から扱う Fiber#resume / mrb_fiber_resume() 関連の問題がいくつか見つかりました。 この PR の範囲では不十分です。 mruby 側で (たぶん僕が) 修正するまでこの PR をドラフト状態にします。

matsumotory commented 12 months ago

了解です。とっても素晴らしい仕事ですね!ありがとうございます!!

dearblue commented 11 months ago

mruby 側の問題が修正されました。 C から Fiber#resume ができることを テストするようにした ので、障害はなくなったと思います (たぶん)。 もっとも、時期的に mruby 3.3 がリリースされても良い頃なので、正式版を待っても良さそうです。 ただし現時点でまだリリース候補が出ていないので、早くても年が明けてからのような気がします・・・・・・。

matsumotory commented 11 months ago

Thanks!

matsumotory commented 11 months ago

ちかいうちにバージョンアップしておきますね

pyama86 commented 11 months ago

ありがとうございます!!!