exit() via exception

4 years ago by Andreas Hennings — view source

unread

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 {}

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

4 years ago by Bruce Weirdan — view source

unread

What other use cases exist for exit()?

Setting exit code for cli scripts.

4 years ago by Mark Randall — view source

unread

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 it is caught, could the exit() potentially be cancelled by not
re-throwing it?

What about a hidden catch block that is inserted at compiler level?

try {
exit();
}
catch (\Throwable) {
...
}

Would become:

try {
exit();
}
catch (\ExitThrowable $ex) {
throw $ex;
}
catch (\Throwable) {
...
}

The hidden catch block would be overwritten with a user catch block if
one existed that caught \ExitThrowable, and it would be required to
re-throw it, otherwise it would be done automatically at the end of the
code segment or upon encountering a return etc.

It's obscure though, it hides things, and I don't think I like it.

Adding another level of throwable is fugly, but it's by far the most
initiative IMO. But take the initiative and prohibit it from being used
by itself so we don't end up in the same position a few years down the line.

try {

}
catch (\TopLevelThrowableForSureThisTime $ex) {

}

^^ Compiler Error.

Mark Randall

4 years ago by Sebastian Bergmann — view source

unread

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.

4 years ago by Claude Pache — view source

unread

Le 11 oct. 2019 à 13:05, Nikita Popov nikita.ppv@gmail.com 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

4 years ago by Thomas Lamy — view source

unread

Am 11.10.19 um 15:16 schrieb Claude Pache:

Le 11 oct. 2019 à 13:05, Nikita Popov nikita.ppv@gmail.com 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

4 years ago by Marcio Almada — view source

unread

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

quit(), exit(), sys.exit() are the equivalent to raise SystemExit, can be caught / interrupted
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

4 years ago by Marcio Almada — view source

unread

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

4 years ago by Nikita Popov — view source

unread

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

quit(), exit(), sys.exit() are the equivalent to raise SystemExit, can be caught / interrupted

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:

Making EngineShutdown implement Throwable, which would make existing
catch(Throwable) catch it -- probably a no-go.
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.
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.
Don't allow catching exits at all. In this case the exception is just an
implementation detail.

Nikita

4 years ago by Aaron Piotrowski — view source

unread

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

quit(), exit(), sys.exit() are the equivalent to raise SystemExit, can be caught / interrupted

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:

Making EngineShutdown implement Throwable, which would make existing
catch(Throwable) catch it -- probably a no-go.

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.

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.

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

4 years ago by Marcio Almada — view source

unread

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:

Making EngineShutdown implement Throwable, which would make existing
catch(Throwable) catch it -- probably a no-go.

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.

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.

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

4 years ago by Nikita Popov — view source

unread

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:

Making EngineShutdown implement Throwable, which would make existing
catch(Throwable) catch it -- probably a no-go.

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.

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.

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

4 years ago by Aaron Piotrowski — view source

unread

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.

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

4 years ago by Bishop Bettini — view source

unread

On Fri, Oct 11, 2019 at 3:47 PM Marcio Almada marcio.web2@gmail.com
wrote:

Em sex, 11 de out de 2019 às 08:05, Nikita Popov
nikita.ppv@gmail.com 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:

quit(), exit(), sys.exit() are the equivalent to raise SystemExit, can be caught / interrupted

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:

Making EngineShutdown implement Throwable, which would make existing
catch(Throwable) catch it -- probably a no-go.

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.

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.

Don't allow catching exits at all. In this case the exception is just an
implementation detail.

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

<?php
register_shutdown_function(fn() => printf("...shutdown"));
try {
exit(13);
} catch (Throwable $t) {
printf("caught %d at %s:%d", $t->getCode(), $t->getFile(),
$t->getLine());
} finally {
printf("...finally");
} fatally { // opt-in: code wants to handle this pathway
printf("...fatally");
}
printf("...outside");
// Outputs: caught 13 at file.php:4...finally...fatally...shutdown
?>

If the fatally branch does not exist, the engine does not pass through the
catch, thus behaving like existing code (no opt-in):

<?php
register_shutdown_function(fn() => printf("...shutdown"));
try {
exit(5);
} catch (Throwable $t) {
printf("caught %d at %s:%d", $t->getCode(), $t->getFile(),
$t->getLine());
} finally {
printf("...finally");
}
printf("...outside");
// Outputs: ...shutdown
?>

Neither Error nor Exception passes through fatally, as would be expected:

<?php
register_shutdown_function(fn() => printf("...shutdown"));
try {
throw new Exception('', 242);
} catch (Throwable $t) {
printf("caught %d at %s:%d", $t->getCode(), $t->getFile(),
$t->getLine());
} finally {
printf("...finally");
} fatally {
printf("...fatally");
}
printf("...outside");
// Outputs: caught 242 at file.php:4...finally...outside
?>

The class hierarchy could then be:

Throwable

Error
Exception
Fatal
- ExitFatal

So you could catch a Fatal (with the fatally branch present) and anything
else if you were so inclined:

<?php
register_shutdown_function(fn() => printf("...shutdown"));
set_error_handler(fn($errno, $errstr) => throw new Exception($errstr,
$errno));
try {
printf(".1f", (float)$argv1 / (float)$argv[2]);
exit("Done");
} catch (DivisionByZeroException | ExitFatal $e) {
printf("...caught %s at %s:%d", $t->getMessage(), $t->getFile(),
$t->getLine());
} finally {
printf("...finally");
} fatally {
printf("...fatally");
}
printf("...outside");
// file.php 4 2
// Outputs: 2.0...caught Done at file.php:6...finally...fatally...shutdown
// file.php 4 0
// Outputs: ...caught "Division by zero" at file.php:5...finally...caught
Done at file.php:6...finally...fatally...shutdown
// file.php
// Outputs: ...finally...caught Done at
file.php:6...finally...fatally...shutdown
?>

I opined maybe exit should be an exception in a 2016 thread1, but the
base motivation was accessing the stack trace so exit points could be
debugged effectively. The ability to trace an exit was welcomed, but making
an exit an exception received some skepticism: an exit-is-an-exit, so it
must act like that.

I think this fatally branch does signal a hard exit, as well as (seemingly)
handling the requirements presented so far:

Unwind the engine gracefully
Opt-in, don't mess with existing catch blocks
Access the exit code and message
Access the exit file and line
Behave in a way consistent with user expectations (vs say catch_exit,
which would be a bit of a one-off compared to other PHP mechanisms).

The name "fatally" may not be ideal, since we have historic "fatal" that
were rewired to "error" in PHP 7. We have contemporary fatal that have no
exception (eg set_memory_limit), but seems to me they should unwind through
this same mechanism. If they did, that would probably complete all the edge
cases currently leading to white pages of death. Eg:

<?php
ini_set('memory_limit', '1M');
try {
// eat a lot of memory
} catch (MemoryLimitFatal $t) {
printf("...caught %s at %s:%d");
} fatally {
printf("...fatally");
}
// Outputs: ...caught Out of memory at file.php:4...fatally
?>

There is the quibble that "If ExitFatal is a Throwable, how come
catch(Throwable) doesn't catch it?", like in my second example:

<?php
register_shutdown_function(fn() => printf("...shutdown"));
try {
exit(5);
} catch (Throwable $t) {
printf("caught %d at %s:%d", $t->getCode(), $t->getFile(),
$t->getLine());
} finally {
printf("...finally");
}
?>

The true, but flippant, answer is "Because BC". The deeper answer is that
Fatal family is only catchable if you signal your willingness to handle
that path way. That signal is the fatally branch. You have to "opt-in" to
the shutdown processing to make the Fatal family catchable. That would lead
to code where the front controller/application dispatch loop would have the
fatally attached, while deeper code would just percolate up as normal.
I.e., I'd only expect to see this fatally branch added in the top-most
entry points, generally speaking.

I've not looked at engine code today, and I have no idea if this
technically feasible. I believe it to be, gut feeling, but don't know. I
can check that later if anyone's interested in this concept.

4 years ago by Olumide Samson — view source

unread

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

4 years ago by Nikita Popov — view source

unread

On Fri, Oct 11, 2019 at 3:47 PM Marcio Almada marcio.web2@gmail.com
wrote:

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

quit(), exit(), sys.exit() are the equivalent to raise SystemExit, can be caught / interrupted

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:

Making EngineShutdown implement Throwable, which would make existing
catch(Throwable) catch it -- probably a no-go.

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.

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.

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 finally, because control flow
performed in finally always wins:

try {
exit;
} finally {
(null)->method(); // throw Error
}

This snippet will throw an Error exception, with the original exit being
discarded.

try {
try {
exit;
} finally {
(null)->method(); // throw Error
}
} catch (Error $e) {
// Ignore
}

This snippet will catch the Error exception and thus stops unwinding.

I believe this behavior is correct and expected (by which I mean:
consistent with finally semantics), but it does mean that exits can be
discarded even without having an explicit "catch_exit" function or similar.

Nikita

4 years ago by Manuel Canga — view source

unread

---- En mié, 11 mar 2020 11:09:13 +0100 Nikita Popov nikita.ppv@gmail.com escribió ----

On Fri, Oct 11, 2019 at 3:47 PM Marcio Almada marcio.web2@gmail.com
wrote:

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

quit(), exit(), sys.exit() are the equivalent to raise SystemExit, can be caught / interrupted

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:

Making EngineShutdown implement Throwable, which would make existing
catch(Throwable) catch it -- probably a no-go.

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.

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.

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 finally, because control flow
performed in finally always wins:

try {
exit;
} finally {
(null)->method(); // throw Error
}

This snippet will throw an Error exception, with the original exit being
discarded.

try {
try {
exit;
} finally {
(null)->method(); // throw Error
}
} catch (Error $e) {
// Ignore
}

This snippet will catch the Error exception and thus stops unwinding.

I believe this behavior is correct and expected (by which I mean:
consistent with finally semantics), but it does mean that exits can be
discarded even without having an explicit "catch_exit" function or similar.

Nikita

Hello,

About this subject:

Nowadays, register_shutdown_function* function exists in order to custom your the end of program. However, with this code:

--- CODE ---

<?php
register_shutdown_function( function() {
echo "\nIn register_shutdown_function";
new Exception("Exception");
});

try {
echo "\nIn Try";
exit();
}catch (\Throwable $e) {
echo "\nIn Catch";
}

--- /CODE ---

Out is:

--- OUT ---

In Try

In register_shutdown_function

--- /OUT ---

Maybe, The solution to exit with exception is that script ends only if all callbacks of register-shutdown-function are ended and It's also necessary that context is keeped. In this way, previous code output should be:

--- OUT ---

In Try

In register_shutdown_function

In Catch

--- /OUT ---

This would keep backward compatibility.

References:

*https://www.php.net/manual/en/function.register-shutdown-function.php

Regards

--
Manuel Canga

4 years ago by Stanislav Malyshev — view source

unread

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
smalyshev@gmail.com