zendtech / ZendOptimizerPlus

Other
914 stars 142 forks source link

Exception handling break when ZO+ enabled and php_sapi_name() method called #195

Closed joe-bowman closed 9 years ago

joe-bowman commented 9 years ago

The following reduced case describes a bug:

if(php_sapi_name() === 'cli') {   // change this to !== 'cli' to replicate under PHP-FPM.
        try { // 1
                try { // 2
                        throw new Exception('Raaaa');
                } catch (Exception $e) { // 2
                        echo 'hello';
                        throw $e;
                }
        } catch (Exception $e1) { // 1
                // squash
        }
}

Expected:

Actual:

This isn't limited to cli mode (also happens under php-fpm), but requires the call to php_sapi_name() and ZO+ 7.0.4-dev to be enabled for the appropriate SAPI.

This worked correctly under 7.0.3-dev. Turning off ZO+ causes the script to behave as expected.

Env: PHP 5.6.3 (PHP-FPM) on RHEL 6.4.

laruence commented 9 years ago

I dont' understand the problem. cli only has one cycle life. how could you do: "On first run after enabling / clearing / invalidating (by updating doc) OpCache, acts as expected. Subsequent runs, the re-thrown exception never gets caught in lower catch logic, and is caught by exception handler instead." ?

thanks

joe-bowman commented 9 years ago

You're right. I've just tested again, and under CLI, with ZO+ enabled this doesn't ever appear to work.

(I think I confused myself trying to cover CLI and FPM cases in one point).

Under PHP-FPM, the expected behaviour is exhibited on the first run, and this issue manifests itself thereafter. Restarting FPM also causes the expected behaviour (presumably because the cache is cleared).

laruence commented 9 years ago

hmm, but in your test script :

if (php_sapi_name() === 'cli')

so nothing should be run in FPM, maybe your test script it wrong?

joe-bowman commented 9 years ago

Under FPM, you have to change the code (!== 'cli' works), but the call to php_sapi_name() is important to replicate the error case.

Sorry if I haven't explained this well.

laruence commented 9 years ago

I tried with php-cgi (opcache enabled)

php56/bin/php-cgi -T 10 test.php

seems works correctly, total 10 times "hello" are outputed

laruence commented 9 years ago

And fpm works also correctly. I refreshed more than 10 times. all have "hello" outputed.

joe-bowman commented 9 years ago

My apologies; I also appear to have XDebug enabled too - turn either XDebug or ZO+ off and it works. I suspect this is just as likely an issue with XDebug then!

joe-bowman commented 9 years ago

I have raised the same issue with XDebug, to see if it's anything their end.

Weirdly (or perhaps not so weirdly) it isn't scoped just the php_sapi_name() method call.

php_uname() php_ini_loaded_file()

both cause the same error also, but only if they are contained within an if(...) statement; called in isolation, these are all fine.

laruence commented 9 years ago

okey, no problem... I am on a task now. so I am not gonna to test with xdebug today.. :)

joe-bowman commented 9 years ago

No problem. I have a workaround for my use case in that I can use PHP_SAPI constant instead of php_sapi_name(), so no rush :)

laruence commented 9 years ago

I confirmed this bug with Derick yesterday. and also got the root cause. will fix it soon, thanks :)

laruence commented 9 years ago

fixed in https://github.com/php/php-src/commit/0547edb5c239a8cbfb7a1d16d85ee365480527dd