Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:107510
MIME-Version: 1.0
References: <CAF+90c9EQ+UGe-y+V_n1QrYkXtPh1uWKXXWEJQUe0xP-MjV7_w@mail.gmail.com>
 <CAOsHV+tpnh9RvbBqXNjs2yp0muu_GHRf3CXVSxg9v081EDGRWQ@mail.gmail.com>
 <CAF+90c_iPDv4RCCG-1vd=cVo-j8GO-BBf=m2KqBQm24xVFwNmg@mail.gmail.com> <2406F2E7-CD65-4E30-A98C-C09FB52B4F2B@trowski.com>
In-Reply-To: <2406F2E7-CD65-4E30-A98C-C09FB52B4F2B@trowski.com>
Date: Fri, 11 Oct 2019 12:13:47 -0300
Message-ID: <CAOsHV+tcNe9ZNzjir7k9G-MmEvZEUcva_f25=f6H68GcC+ZwAg@mail.gmail.com>
To: Aaron Piotrowski <aaron@trowski.com>
Cc: Nikita Popov <nikita.ppv@gmail.com>, PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] exit() via exception
From: marcio.web2@gmail.com (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 us=
able
> > in a webserver context, while a hypothetical pcntl_exit() would bring d=
own
> > the server process. As the Python docs mention, the primary use-case wo=
uld
> > 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 all=
ows
> > explicitly catching the exit.
> >
> > 3. Introducing a function like catch_exit(function() { ... }). This wou=
ld
> > still allow catching exits (for phpunit + daemon use cases), but the fa=
ct
> > 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 jus=
t 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 ins=
tigation in user-land.
> https://github.com/php/php-src/blob/db233501ff9d56765ef4a870b777a643c2136=
711/Zend/zend_exceptions.c#L909-L916
>
> No catch block would catch it, because it wouldn't implement Throwable no=
r 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=C3=A1rcio