Make exit() unwind properly

[nikic]

Exit() is not internally modeled as an exception, but not exposed to userland. This means that the stack will be properly unwound and there will be no memory leaks.

Additionally, this enables running of finally blocks on exit:

try {
    exit("Exit\n");
} finally {
    echo "Finally\n";
}

// Prints:
Finally
Exit

Note that the exit message is not printed immediately, but only once the exit reaches the top-most frame. I believe it has to work this way if we ever want to make exits catchable in some form (e.g. via a special catch_exit function).

[morrisonlevi]

Why does phpdbg still need the bailout?

[nikic]

Because I have no idea how to handle it properly ... maybe @bwoebi knows?

[bwoebi]

Do we need special handling at all here?

at least you've removed EG(exit_status) - so have a look at phpdbg.c:1963 and retrieve the exit state from the exception?

[SammyK]

\0/ Burn it with 🔥! 😄

[ghost]

Do we actually need/want to run finally blocks? My understanding is that when calling exit no further PHP code is executed and the PHP engine goes into shutdown, which leads to internal cleanup, stack unwind and then a clean process exit.

Edit: To expand my comment a little...

I am talking in the context of the CLI SAPI.

If the finally blocks were executed it could have unintended side effects. exit could suddenly not mean anymore exit now. Since code will be executed, the shutdown could be delayed massively.

If I'm 10 stack frames deep down in a function call and each frame has a finally blocks that access some blocking part (NFS, database, etc.), then it wont exit now, but later when all finally blocks have been executed.

Now imagine this piece of code:

try {
    exit;
} finally {
    while(true);
}

What will happen now if finally blocks were executed? I'd imagine an exit will never happen, because we have an infinite loop. Now even an once guaranteed exit will never happen anymore.

I know the example is ridiculous, but it should visualize that if I want to exit but run all finally blocks, there are ways to do that, exit's behaviour should not be changed. It's by any means "a last effort to exit at all".

[nikic]

@CharlotteDunois Nothing will materially change, because exit currently already results in the execution of shutdown handlers and destructors. "Finally" is basically a different form of writing RAII destructors.

[nikic]

To make it clearer what I mean:

<?php

class OnScopeExit {
    private Closure $callback;
    public function __construct(Closure $callback) {
        $this->callback = $callback;
    }
    public function __destruct() {
        ($this->callback)();
    }
}

function foo() {
    try {
        // Try
    } finally {
        // Finally
    }
}

function bar() {
    $_ = new OnScopeExit(function() {
        // Finally
    });
    // Try
}

foo() and bar() are basically two ways to write the same thing (glossing over a lot of details). Right now for exit the destructor version does run, while the finally version does not run. This is of course quite bad if your finally block contains something like unlock_shm();.

[ghost]

@nikic Do I interpret the Zend/tests/exit_finally_2.phpt test correctly that with this change an exit instruction can be "caught"? Looking at the expected output shows that the exit output is missing, thus the exit got actually caught/overwritten by the throw new Exception instruction?

[nikic]

I've opened #5768 as a version of this with reduced scope. Let's get the basic unwinding in first and discuss finally blocks separately.

[ramsey]

It looks like the version of this with reduced scope (#5768) was merged. Is it time to revisit this, or are the finally blocks being dealt with elsewhere?

[github-actions]

There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken.