RE: [PHP-CVS] com php-src: Fix bug #73100 - Improve bug fix. Forbid to set 'user' save handler other than set_save_handler().:
ext/session/php_session.h ext/session/session.c ext/session/tests/bug60860.phpt ext/session/tests/bug73100.phpt ext/session/test

Hi Yasuo, > -----Original Message----- > From: Anatol Belski [mailto:anatol.php@belski.net] > Sent: Monday, December 26, 2016 10:13 PM > To: 'Yasuo Ohgaki' <yohgaki@ohgaki.net> > Cc: php-cvs@lists.php.net > Subject: RE: [PHP-CVS] com php-src: Fix bug #73100 - Improve bug fix. Forbid to > set 'user' save handler other than set_save_handler().: > ext/session/php_session.h ext/session/session.c > ext/session/tests/bug60860.phpt ext/session/tests/bug73100.phpt > ext/session/test > > Hi Yasuo, > > > -----Original Message----- > > From: Yasuo Ohgaki [mailto:yohgaki@ohgaki.net] > > Sent: Friday, December 23, 2016 1:42 AM > > To: Anatol Belski <anatol.php@belski.net> > > Cc: Yasuo Ohgaki <yohgaki@php.net>; php-cvs@lists.php.net > > Subject: Re: [PHP-CVS] com php-src: Fix bug #73100 - Improve bug fix. > > Forbid to set 'user' save handler other than set_save_handler().: > > ext/session/php_session.h ext/session/session.c > > ext/session/tests/bug60860.phpt ext/session/tests/bug73100.phpt > > ext/session/test > > > > Hi Anatol, > > > > On Thu, Dec 22, 2016 at 10:13 PM, Anatol Belski > > <anatol.php@belski.net> > > wrote: > > > Hi Yasuo, > > > > > >> -----Original Message----- > > >> From: Yasuo Ohgaki [mailto:yohgaki@php.net] > > >> Sent: Thursday, December 22, 2016 8:04 AM > > >> To: php-cvs@lists.php.net > > >> Subject: [PHP-CVS] com php-src: Fix bug #73100 - Improve bug fix. > > >> Forbid to set 'user' save handler other than set_save_handler().: > > >> ext/session/php_session.h ext/session/session.c > > >> ext/session/tests/bug60860.phpt ext/session/tests/bug73100.phpt > > >> ext/session/tests/se > > >> > > >> Commit: a93a51c3bf4ea1638ce0adc4a899cb93531b9f0d > > >> Author: Yasuo Ohgaki <yohgaki@php.net> Thu, 22 Dec 2016 15:57:53 > > >> +0900 > > >> Parents: d4831e2d0a812486cae8fb30afa5b9e0e03e03ce > > >> Branches: master > > >> > > >> Link: http://git.php.net/?p=php- > > >> src.git;a=commitdiff;h=a93a51c3bf4ea1638ce0adc4a899cb93531b9f0d > > >> > > >> Log: > > >> Fix bug #73100 - Improve bug fix. Forbid to set 'user' save handler > > >> other than set_save_handler(). > > >> > > > Some tests are failing with AppVeyor run, could you please check? > > > > Thank you for let me know. > > It seems I have to set INI value. I'll fix it soon. > Thanks for checking. I forked those two, it's a test bug due to the system I/O > scheduling, probably would be hard to fix another way. > There is still a failure in the TS run, even I've changed the test https://ci.appveyor.com/project/php/php-src/build/master.build.152/job/laptrm6y031e11k3#L5606 It seems to me, there's an issue with the rev a93a51c3bf4ea1638ce0adc4a899cb93531b9f0d and the module initialization order at least in the thread safe build. I did a short debug run, and it shows, that in some case the E_RECOVERABLE_ERROR won't have any effect in OnUpdateSaveHandler, or generally - in some INI handler. It depends on the module init order. The related code is here https://php-lxr.adamharvey.name/source/xref/master/main/main.c#1208 One can see, that the error handler won't bail out, if the full module initialization is incomplete. AFM, it tells, that throwing such errors in the MINIT phase is not reliable. As the test relies on the --INI-- section, that's the reason the exact behavior. IMO, if such error happens in the MINIT phase, an error might be unreliable, for some reason the session module still gets initialized :(. Maybe, the actual check should move into the later code, or there's a bug elsewhere. Could you please take a further look? I'll be able to turn to this probably next year at earliest. Thanks Anatol