v2.2.3 build failing at t/50mruby.t

[apoikos]

Hi,

I'm trying to build v2.2.3, but it looks like the recent mruby merge broke t/50mruby.t. The following mruby handler defined at t/50mruby.t:241 does not work as expected:

        mruby.handler: |
          cnt = 0
          Proc.new do |env|
            cnt += 1
            if cnt % 2 != 0
              [200, {}, ["hello\n"]]
            else
              raise "error from rack"
            end
          end

Adding some debug prints, cnt += 1 seems not to work (!?). The value of cnt is always 0, causing the handler to raise an error on every request. This affects the Travis build as well.

Regards,
Apollon

[tatsushid]

This is the issue what I met. This and the test case 10 in 50mruby.t both use a value outside of the Proc context and it is referred by the Proc. As you mentioned, the value doesn't work at all in Proc.

This is the issue introduced by upgrading mruby 1.3 I guess and this is similar to mruby issue mruby/mruby#3819. Actually, I could clone the same behavior on mirb with the test code and it has been fixed by the commit mruby/mruby@1988bec (I found it by git bisect with the test code).

To fix it, I propose adding the same fix to lib/handler/mruby.c at line 154

[apoikos]

Indeed, the patch seems to work. Thanks!

[kazuho]

@apoikos @tatsushid Thank you very much for reporting and finding the way to fix the issue.

I have applied the proposed fix in 0afbe82 and awaiting CI.

Interestingly, MRB_ENV_STACK_LEN(e) < proc->body.irep->nlocals never becomes true on my MacBook Pro (macOS 10.12, x86-64) or on my Ubuntu 16.04 (x86-64) VM.

[kazuho]

The test output of https://travis-ci.org/h2o/h2o/builds/290288124 shows that the issue has been fixed. The remaining test failure is a known-issue related to the default PHP executable on Trusty, which has been resolved in master.

Thank you for the quick report and the fix.