> On Oct 11, 2019, at 9:11 AM, Nikita Popov <nikita....@gmail.com> wrote:
> 
> On Fri, Oct 11, 2019 at 3:47 PM Marcio Almada <marcio.w...@gmail.com> wrote:
> 
>> Em sex, 11 de out de 2019 às 08:05, Nikita Popov
>> <nikita....@gmail.com> escreveu:
>>> 
>>> Hi,
>>> 
>> 
>> Hello :)
>> 
>>> Currently exit() is implemented using bailout and unclean shutdown, which
>>> means that we're going to perform a longjmp back to the top-level scope
>> and
>>> let the memory manager clean up all the memory it knows about. Anything
>> not
>>> allocated using ZMM is going to leak persistently.
>>> 
>>> For me, one of the most annoying things about this is that we can't
>> perform
>>> proper leak checks on code using PhpUnit, because it will always exit()
>> at
>>> the end, which will result in "expected" memory leaks.
>>> 
>>> I think it would be good to switch exit() to work by throwing a magic
>>> exception, similar to what Python does. This would allow us to properly
>>> unwind the stack, executing finally blocks (which are currently skipped)
>>> and perform a clean engine shutdown.
>>> 
>>> Depending on the implementation, we could also allow code to actually
>> catch
>>> this exception, which may be useful for testing scenarios, as well as
>>> long-running daemons.
>>> 
>>> I'm mainly wondering how exactly we'd go about integrating this in the
>>> existing exception hierarchy.
>> 
>>> Assuming that it is desirable to allow people
>>> to actually catch this exception
>>> my first thought would be along these
>>> lines:
>>> 
>>> Throwable (convert to abstract class)
>>> \-> Exception
>>> \-> Error
>>> \-> ExitThrowable
>>> 
>>> This does mean though that existing code using catch(Throwable) is going
>> to
>>> catch exit()s as well. This can be avoided by introducing *yet another*
>>> super-class/interface above Throwable, which is something I'd rather
>> avoid.
>>> 
>> 
>> Since you brought python as inspiration, I believe the hierarchy goes
>> like this on their land:
>> 
>> BaseException
>> +-- SystemExit
>> +-- KeyboardInterrupt
>> +-- GeneratorExit
>> +-- Exception
>>     +-- [kitchen sink]
>> 
>> Being `BaseException` the base class for all built-in exceptions. It
>> is not meant to be directly
>> inherited by user-defined classes. It 's the equivalent to our
>> `Throwable` situation. In this context
>> `ExitThrowable -> Throwable ` appears legit.
>> 
>>> 
>>> Anyone have thoughts on this matter?
>>> 
>> 
>> Yes. There is an obvious can of worms if I've got this right: `exit()`
>> and `die()` would no longer guarantee a
>> program to actually terminate in case catching `ExitThrowable` is
>> allowed. Python solves this by actually
>> having two patterns:
>> 
>> 1. `quit()`, `exit()`, `sys.exit()` are the equivalent to `raise
>> SystemExit`, can be caught / interrupted
>> 2. `os._exit()`, can't be caught but has a callback mechanism like our
>> `register_shutdown_function`,
>> see https://docs.python.org/3/library/atexit.html
> 
> 
> I don't believe atexit applies to os._exit(). In any case, I agree that
> this is something we're currently missing -- we should probably add a
> pcntl_exit() for this purpose. It should be noted though that this is
> really very different from exit(), which is still quite graceful and usable
> in a webserver context, while a hypothetical pcntl_exit() would bring down
> the server process. As the Python docs mention, the primary use-case would
> be exiting from forked processes without going through shutdown, which has
> also recently come up in https://github.com/php/php-src/pull/4712.
> 
> 
>> If we bind `exit()` and `die()` to a catchable exception how would we
>> still have the scenario 2 available
>> on PHP land without a BCB? :)
>> 
> 
>> I have one simple suggestion: Introduce `EngineShutdown -> Throwable`,
>> bind `exit|die` to it but disallow
>> `catch(\EngineShutdown $e)` at compile time. This would allow keeping
>> backwards compatibility to
>> scenario 2 without messing with our current exception hierarchy.
>> 
> 
> I think the options are basically:
> 
> 1. Making EngineShutdown implement Throwable, which would make existing
> catch(Throwable) catch it -- probably a no-go.
> 
> 2. Making EngineShutdown not implement Throwable, which means that not all
> "exceptions" implement the interface, which is rather odd. It still allows
> explicitly catching the exit.
> 
> 3. Introducing a function like catch_exit(function() { ... }). This would
> still allow catching exits (for phpunit + daemon use cases), but the fact
> that this is actually implemented based on an exception would be hidden and
> the only way to catch the exit is through this function.
> 
> 4. Don't allow catching exits at all. In this case the exception is just an
> implementation detail.
> 
> Nikita

+1 for option 3.

EngineShutdown could be a special exception to the engine, being handled like 
an exception internally, but not implement Throwable and therefore not an 
exception from user-land's point-of-view.

EngineShutdown could be added to the list of "throwables", but forbid 
instigation in user-land.
https://github.com/php/php-src/blob/db233501ff9d56765ef4a870b777a643c2136711/Zend/zend_exceptions.c#L909-L916

No catch block would catch it, because it wouldn't implement Throwable nor 
extend Exception or Error.

Aaron Piotrowski

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to