exit() via exception

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:

1. quit(), exit(), sys.exit() are the equivalent to raise SystemExit, can be caught / interrupted
2. os._exit(), can't be caught but has a callback mechanism like our
   register_shutdown_function,
   see https://docs.python.org/3/library/atexit.html

I don't believe atexit applies to os._exit(). In any case, I agree that
this is something we're currently missing -- we should probably add a
pcntl_exit() for this purpose. It should be noted though that this is
really very different from exit(), which is still quite graceful and usable
in a webserver context, while a hypothetical pcntl_exit() would bring down
the server process. As the Python docs mention, the primary use-case would
be exiting from forked processes without going through shutdown, which has
also recently come up in https://github.com/php/php-src/pull/4712.

If we bind exit() and die() to a catchable exception how would we
still have the scenario 2 available
on PHP land without a BCB? :)

I have one simple suggestion: Introduce EngineShutdown -> Throwable,
bind exit|die to it but disallow
catch(\EngineShutdown $e) at compile time. This would allow keeping
backwards compatibility to
scenario 2 without messing with our current exception hierarchy.

I think the options are basically:

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

2. Making EngineShutdown not implement Throwable, which means that not all
   "exceptions" implement the interface, which is rather odd. It still allows
   explicitly catching the exit.

3. Introducing a function like catch_exit(function() { ... }). This would
   still allow catching exits (for phpunit + daemon use cases), but the fact
   that this is actually implemented based on an exception would be hidden and
   the only way to catch the exit is through this function.

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

Nikita

+1 for option 3.

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

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

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

Aaron Piotrowski

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:

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:

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

1. Unwind the engine gracefully
2. Opt-in, don't mess with existing catch blocks
3. Access the exit code and message
4. Access the exit file and line
5. 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.

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:

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