Re: exit() via exception

> On Oct 11, 2019, at 9:11 AM, Nikita Popov <nikita.ppv@gmail.com> wrote: > > 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. > > 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