Re: [PHP-DEV] exit() via exception

[Manuel Canga]

  En mié, 11 mar 2020 11:09:13 +0100 Nikita Popov  
escribió 
 > On Fri, Oct 11, 2019 at 4:11 PM Nikita Popov  wrote:
 > 
 > > On Fri, Oct 11, 2019 at 3:47 PM Marcio Almada 
 > > wrote:
 > >
 > >> Em sex, 11 de out de 2019 às 08:05, Nikita Popov
 > >>  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 

Re: [PHP-DEV] exit() via exception

[Nikita Popov]

On Fri, Oct 11, 2019 at 4:11 PM Nikita Popov  wrote:

> On Fri, Oct 11, 2019 at 3:47 PM Marcio Almada 
> wrote:
>
>> Em sex, 11 de out de 2019 às 08:05, Nikita Popov
>>  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.
>

I've started implementing variant 4 in
https://github.com/php/php-src/pull/5243. It uses an internal exception
type to implement exit() that cannot be caught.

Finally blocks do get executed, as they should be. This does mean that it
is possibly to discard the exit through 

Re: [PHP-DEV] exit() via exception

[Thomas Lamy]

Am 11.10.19 um 15:16 schrieb Claude Pache:



Le 11 oct. 2019 à 13:05, Nikita Popov  a écrit :

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.


We should keep the semantics of `exit`, which is quite akin an early `return` 
in a function. Therefore:

* Executing finally blocks (as does an early `return`) would be an improvement.

* But intercepting it in a `catch` block or in an exception handler, is usually 
unwanted, contrarily to everything that currently implements `Throwable`.

But if you do want to allow exit signals to be caught in catch blocks, there is 
no absolute necessity to introduce yet another superclass or superinterface, 
because you can write (since PHP 7.1):

catch (\Throwable | \ExitSignal $e)

—Claude


This! +1

Tom

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

Re: [PHP-DEV] exit() via exception

[Stanislav Malyshev]

Hi!

> 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.

Is that something that might be fixed in phpunit? I am not familiar with
this specific issue but I'm not sure why unit test code can't use some
other way to end whatever it's doing than exit() if that's the issue. Is
it about the exit codes? If so, this probably can be fixed by other means?

> 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.

True, but that means exit() would become a) significantly slower b) may
change semantics, which may or may not be a good thing. Also there's a
possibility of exit() failing then which is not something we've had before.

> 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 don't think this is a particularly good idea - first of all, using
exception for flow control is wrong. Second of all, if you _want_ to use
exceptions for flow control, you already can. If the code uses exit(),
it usually means exit, as in drop everything and get the heck out. It
may not expect that lots of code will run after that (yes, I know,
shutdown handlers, but they have to be clearly installed as such) that
may still do a lot of stuff - while the state of the app is potentially
broken. If we make exit catchable, then the next request would be to
implement real, un-catchable, exit that actually implements the old
semantics. Sometimes people don't care about memory leaks checking but
want to just abandon the boat and let the memory manager to clean up the
mess (or even not that, just kill the process and be done).

> 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:

I don't think it should be Throwable, since you a) can't and shouldn't
actually throw it and b) code that catches Throwable does not expect to
catch exits, so it would break its semantics. Granted, there's almost
never is the reason to catch Throwable, but if you already do, you'd be
in for a nasty surprise.

> 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.

If you want to do weird things - like have exception that it's really
not an exception - you'd have weird hierarchy. Either that, or you make
existing hierarchy weird, which existing code (and virtually everybody
writing new code) does not expect. I think theoretical weirdness is
better than nasty surprise for the practical users - which would either
have to insert instanceof checks into their catches, or deal with exit()
behaving wrongly.

-- 
Stas Malyshev
smalys...@gmail.com

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

Re: [PHP-DEV] exit() via exception

[Olumide Samson]

I'm thinking exit() shouldn't be catchable to maintain status quo, and it
should be focused on the reason it was suggested(Unwinding stacks and
cleaning up memories instead of longjmp'ing to shutdown).

If there's any need to catch it's exception, that can be handled later
through maybe a RFC discussion.

This can be implemented directly without having any user land interaction
since the throwing and catching can't be caught by any user land
code(top-most hierarchy without possibility to be caught, which might
result in compile time error).

All the best

Re: [PHP-DEV] exit() via exception

[Bishop Bettini]

On Fri, Oct 11, 2019 at 10:11 AM Nikita Popov  wrote:

> On Fri, Oct 11, 2019 at 3:47 PM Marcio Almada 
> wrote:
>
> > Em sex, 11 de out de 2019 às 08:05, Nikita Popov
> >  escreveu:
> >
> > > 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.
>

5. A new branch in the try...catch...finally model, which signals your
willingness to handle a fatal pathway:

 printf("...shutdown"));
try {
exit(13);
} catch (Throwable $t) {
printf("caught %d at %s:%d", 

Re: [PHP-DEV] exit() via exception

[Aaron Piotrowski]

> On Oct 11, 2019, at 10:21 AM, Nikita Popov  > wrote:
> 
>> Hi!
>> 
>> So maybe it narrows down to:
>> 
>> Is there an essencial attempt to improve `exit()` handling from the
>> userland perspective or should the focus be solely on solving the
>> memory management issue pointed out in the beginning of the thread?
>> 
>> If the scope is to also improve userland, option 3 could be the way to
>> go indeed but I confess to do not be a fan of another callback
>> registering thing... it feels hard to predict when you consider:
>> 
>> ```
>> catch_exit(function(){
>>exit(); // what happens here? We still have to guarantee `exit` to
>> halt at some point.
>> });
>> ```
>> 
>> And what are the interactions with `register_shutdown_function`? I
>> suppose the `catch_exit` stack has to be run before the
>> `register_shutdown_function` stack? Considering the behavior in the
>> docs.
>> 
> 
> I think I was a bit unclear in how the catch_exit() function is intended to
> work: It's not an atexit handler, it's basically a try/catch block for
> exits.
> 
> $exitExceptionOrNull = catch_exit(function() {
>// Run code that may contain exit() here
> });
> 
> or possibly even more explicitly as:
> 
> catch_exit(function() {
>// Run code that may contain exit() here
> }, function($exitCode, $exitMessage) {
>// This is called if an exit() occurred
> });

The second option seems better, as it's a lot more obvious what code will be 
executed if exit() is called and what will not be.

Would a set_exit_handler function be possible, similar to set_exception_handler?

> 
> I like option 4 much more for now. It allows tackling the root issue
>> and still leaves possibilities open regarding how the exception
>> hierarchy could be and how the handling of `exit` could happen
>> (through a catch at userspace or callback registering).
>> 
> 
> I guess we should do that as the first step in any case ... everything else
> would be extensions on top of that, but this would be the main technical
> groundwork.
> 
> Nikita

Option 4 of course would be fine for now. Once that's done, we can decide how 
exits could be "caught" in the future.


> On Fri, Oct 11, 2019 at 5:13 PM Marcio Almada  > wrote:
> 
>>> 
>>> 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.
>>> 
>> 
>> Very elegant solution!
>> 
>> PS: Naming things is hard, but `Throwable` could not have been a
>> better choice in retrospect. Ty ;)

Thanks! Every once-in-a-while I manage to name something correctly!

>> 
>>> Aaron Piotrowski
>>> 
>> 
>> Márcio

Aaron Piotrowski

Re: [PHP-DEV] exit() via exception

[Bruce Weirdan]

On Fri, Oct 11, 2019 at 2:43 PM Andreas Hennings  wrote:

> What other use cases exist for exit()?

Setting exit code for cli scripts.

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

Re: [PHP-DEV] exit() via exception

[Nikita Popov]

On Fri, Oct 11, 2019 at 5:13 PM Marcio Almada  wrote:

> Hi!
>
> > > 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.
>
> So maybe it narrows down to:
>
> Is there an essencial attempt to improve `exit()` handling from the
> userland perspective or should the focus be solely on solving the
> memory management issue pointed out in the beginning of the thread?
>
> If the scope is to also improve userland, option 3 could be the way to
> go indeed but I confess to do not be a fan of another callback
> registering thing... it feels hard to predict when you consider:
>
> ```
> catch_exit(function(){
> exit(); // what happens here? We still have to guarantee `exit` to
> halt at some point.
> });
> ```
>
> And what are the interactions with `register_shutdown_function`? I
> suppose the `catch_exit` stack has to be run before the
> `register_shutdown_function` stack? Considering the behavior in the
> docs.
>

I think I was a bit unclear in how the catch_exit() function is intended to
work: It's not an atexit handler, it's basically a try/catch block for
exits.

$exitExceptionOrNull = catch_exit(function() {
// Run code that may contain exit() here
});

or possibly even more explicitly as:

catch_exit(function() {
// Run code that may contain exit() here
}, function($exitCode, $exitMessage) {
// This is called if an exit() occurred
});

I like option 4 much more for now. It allows tackling the root issue
> and still leaves possibilities open regarding how the exception
> hierarchy could be and how the handling of `exit` could happen
> (through a catch at userspace or callback registering).
>

I guess we should do that as the first step in any case ... everything else
would be extensions on top of that, but this would be the main technical
groundwork.

Nikita


> >
> > 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.
> >
>
> Very elegant solution!
>
> PS: Naming things is hard, but `Throwable` could not have been a
> better choice in retrospect. Ty ;)
>
> > Aaron Piotrowski
> >
>
> Márcio
>

Re: [PHP-DEV] exit() via exception

[Marcio Almada]

Hi!

> > 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.

So maybe it narrows down to:

Is there an essencial attempt to improve `exit()` handling from the
userland perspective or should the focus be solely on solving the
memory management issue pointed out in the beginning of the thread?

If the scope is to also improve userland, option 3 could be the way to
go indeed but I confess to do not be a fan of another callback
registering thing... it feels hard to predict when you consider:

```
catch_exit(function(){
exit(); // what happens here? We still have to guarantee `exit` to
halt at some point.
});
```

And what are the interactions with `register_shutdown_function`? I
suppose the `catch_exit` stack has to be run before the
`register_shutdown_function` stack? Considering the behavior in the
docs.

I like option 4 much more for now. It allows tackling the root issue
and still leaves possibilities open regarding how the exception
hierarchy could be and how the handling of `exit` could happen
(through a catch at userspace or callback registering).

>
> 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.
>

Very elegant solution!

PS: Naming things is hard, but `Throwable` could not have been a
better choice in retrospect. Ty ;)

> Aaron Piotrowski
>

Márcio

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

Re: [PHP-DEV] exit() via exception

[Aaron Piotrowski]

> On Oct 11, 2019, at 9:11 AM, Nikita Popov  wrote:
> 
> On Fri, Oct 11, 2019 at 3:47 PM Marcio Almada  wrote:
> 
>> Em sex, 11 de out de 2019 às 08:05, Nikita Popov
>>  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 

Re: [PHP-DEV] exit() via exception

[Nikita Popov]

On Fri, Oct 11, 2019 at 3:47 PM Marcio Almada  wrote:

> Em sex, 11 de out de 2019 às 08:05, Nikita Popov
>  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

Re: [PHP-DEV] exit() via exception

[Marcio Almada]

> 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.
>
> > Nikita
>

Sorry, in the latest message I meant introducing `EngineShutdown` without
extending `Throwable`:

> Thanks,
> Márcio

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

Re: [PHP-DEV] exit() via exception

[Marcio Almada]

Em sex, 11 de out de 2019 às 08:05, Nikita Popov
 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

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.

> Nikita

Thanks,
Márcio

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

Re: [PHP-DEV] exit() via exception

[Claude Pache]

> Le 11 oct. 2019 à 13:05, Nikita Popov  a écrit :
> 
> 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.


We should keep the semantics of `exit`, which is quite akin an early `return` 
in a function. Therefore:

* Executing finally blocks (as does an early `return`) would be an improvement.

* But intercepting it in a `catch` block or in an exception handler, is usually 
unwanted, contrarily to everything that currently implements `Throwable`.

But if you do want to allow exit signals to be caught in catch blocks, there is 
no absolute necessity to introduce yet another superclass or superinterface, 
because you can write (since PHP 7.1): 

catch (\Throwable | \ExitSignal $e)

—Claude

Re: [PHP-DEV] exit() via exception

[Sebastian Bergmann]

Am 11.10.2019 um 13:05 schrieb Nikita Popov:

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.


This sounds interesting :)


Anyone have thoughts on this matter?


I think \Throwable should always include all objects that can be used with 
throw and catch.


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

Re: [PHP-DEV] exit() via exception

[Andreas Hennings]

I would think that whichever code currently calls exit() is already
expecting and accepting all the consequences, and assumes that this
won't ever be caught.

So, would it be an option to keep exit() as it is, and introduce a new
syntax construction?

Places where I have seen exit() were not really about exceptional
emergency situations, but rather a "finished" or "done".
E.g. "send headers, print ajax response, exit".
This may or may not be good practice, but it is something being done.

What other use cases exist for exit()?

> introducing *yet another* super-class/interface above Throwable, which is 
> something I'd rather avoid.

I don't see a way to avoid it ..
But if we are going to do this, we should think about whether we
really covered all cases, or if we need another layer in the future.

interface Throwable extends Catchable {}


On Fri, 11 Oct 2019 at 13:05, Nikita Popov  wrote:
>
> Hi,
>
> 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.
>
> Anyone have thoughts on this matter?
>
> Nikita

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