Hey Larry, Tim, Seifeddine and Arnauld,
On 4.11.2025 21:13:18, Larry Garfield wrote:
Arnaud and I would like to present another RFC for consideration: Context
Managers.
https://wiki.php.net/rfc/context-managers
You'll probably note that is very similar to the recent proposal from Tim and
Seifeddine. Both proposals grew out of casual discussion several months ago; I
don't believe either team was aware that the other was also actively working on
such a proposal, so we now have two. C'est la vie. :-)
Naturally, Arnaud and I feel that our approach is the better one. In
particular, as Arnaud noted in an earlier reply, __destruct() is unreliable if
timing matters. It also does not allow differentiating between a success or
failure exit condition, which for many use cases is absolutely mandatory (as
shown in the examples in the context manager RFC).
The Context Manager proposal is a near direct port of Python's approach, which
is generally very well thought-out. However, there are a few open questions as
listed in the RFC that we are seeking feedback on.
Discuss. :-)
I've been looking at both RFCs and I don't think either RFC is good
enough yet.
As for this RFC:
It makes it very easy to not call the exitContext() method when calling
enterContext() manually. The language (obviously) doesn't prevent
calling enterContext() - and that's a good thing. But also, it will not
enforce that exitContext() gets ever called (and it also cannot,
realistically).
Thus, we have a big pitfall, wherein APIs may expect enterContext() and
exitContext() to be called in conjunction, but users don't - with
possibly non-trivial side-effects (locks not cleared, transactions not
committed etc.). Thus, to be safe, implementers of the interface will
also likely need the destructor to forward calls to exitContext() as
well. But it's an easy thing to forget - after all, the *intended* usage
of the API just works. Why would I think of that, as an implementer of
the interface, before someone complains?
Ultimately you definitely will need the capability of calling
enterContext() and exitContext() manually too (i.e. restricting that is
not realistic either), as lifetimes do not necessarily cleanly nest - as
a trivial example, you might want to obtain access to a handle which is
behind a lock. You'll have to enter the context of the lock, enter the
context of the handle, and close the lock (because more things are
locked behind that lock, including the handle). ... But you don't
necessarily want the hold on the lock to outlive the inner handle. In
short: The proposed approach only allows nesting contexts, but not
interleaving them.
Further, calling with() twice on an object is quite bad in general. But
it might easily happen - you have a function which wants a transaction.
e.g. function writeSomeData(DatabaseTransaction $t) { with ($t) {
$t->query("..."); } }. A naive caller might think, DatabaseTransaction
implements ContextManager ... so let's wrap it: with($db->transaction()
as $t) { writeSomeData($t); }. But now you are nesting a transaction,
which may have unexpected side effects - and the code probably not
prepared to handle it. So, you have to add yet another safeguard into
your implementation: check whether enterContext() is only active once.
... Or, maybe a caller assumes that $t = $db->transaction(); with ($t) {
$t->query("..."); } with ($t) { $t->query("..."); } is fine - but the
implementation is not equipped to handle multiple calls to enterContext().
Additionally, I would expect implementers to want to provide methods,
which can be called while the context is active. However, it's not
impossible to call these methods without wrapping it into with() or
calling the enterContext() method explicitly. One more failure mode,
which needs handling.
Like for example, calling $t->query() on a transaction without starting it.
I don't like that design, which effectively forces you to put safety
checks for all but the simplest cases onto the ContextManager
implementation.
And it forces the user to recognize "this returned object
DatabaseTranscation actually implements ContextManager, thus I should
put it into with() and not immediately call methods on it". (A problem
which the use() proposal from Tim does not have by design.)
The choice of adding the exception to the exitContext() is interesting,
but also very opinionated:
- It means, that the only way to abort, in non-exceptional cases, is to
throw yourself an exception. And put a try/catch around the with() {}
block. Or manually use enterContext() & exitContext() - with a fake "new
Exception" essentially.
- Maybe you want to hold a transaction, but just ensure that everything
gets executed together (i.e. atomicity), but not care about whether
everything actually went through (i.e. not force a rollback on
exception). You'll now have to catch the exception, store it to a
variable, use break and check for the exception after the with block.
Or, yes, manually using enterContext() and exitContext().
It feels like with() is designed to be covering 70% of the use cases,
with a load of hidden pitfalls and advanced usage requiring manual
enterContext() and exitContext() calls. It's not a very good solution.
As to the destructors (and also in reply to that other email from
Arnauld talking about PDO::disconnect() etc.):
It's already possible today to have live objects which are already
destructed. It's extremely common to have in shutdown code. It's
sometimes a pain, I agree. But it's an already known pain, and an
already handled pain in a lot of code.
If your object only knows "create" and "destruct", there's no way for a
double enterContext() (nested or consecutive) situation to ever happen.
(Well, yes, you *could* theoretically manually call __destruct(), but
why would you ever do that?)
Last thing - proper API usage forces you to use that construct.
To the use() proposal from Tim:
This proposal makes it very simple to inadvertently leak the use()'d
value. I don't think the current proposed form goes far enough.
However we could decide to force-destruct an object (just like we do in
shutdown too). It's just one single flag for child methods to check as
well - the object is either destructed or not. We could also trivially
prohibit nested use() calls by throwing an AlreadyDestructedError when
an use()'d and inside destructed object crosses the use() boundary.
The only disadvantage is that there's no information about thrown
exceptions. I.e. you cannot add a default behaviour of "on exception,
please do this", like rolling transactions back. But:
- Is it actually a big problem? Where is the specific disadvantage over
simply $db->transaction(function($t) { /* do stuff */ }); - where the
call of the passed Closure can be trivially wrapped in try/catch.
- If yes, can we e.g. add an interface ExceptionDestructed { public bool
$destructedDuringException; }? Which will set that property if the
property is still undefined - to true if the destructor gets triggered
inside ZEND_HANDLE_EXCEPTION. To false otherwise. And, if an user
desires to manually force success/failure handling, he may set
$object->destructedDuringException = true; himself as a very simple -
one-liner - escape hatch.
The use() proposal is not a bad one, but I feel like requiring the RC to
drop to zero first, misses a bit of potential to save users from mistakes.
The other nice thing about use() is that it's optional. You don't have
to use it. You use it if you want some scoping, otherwise the scope is
simply the function scope.
To both proposals:
It remains possible by default to call methods on the object, after
leaving the with or use block. So some checking on methods for a safe
API is probably still required.
I don't think it's possible to solve that problem at all with the
ContextManager RFC, except manual checking by the implementer in every
single method. But it's possibly possible to solve it with the use() RFC
in orthogonal ways - like a #[ProhibitDestructed] attribute, which can
be added onto a class (making it apply to all methods) or a specific
method and causes method calls to throw an exception when the object is
destructed.
Which is possible to provide by the language, as the language knows
about whether objects are already destructed, unlike e.g. the
ContextManager, where it would be object state, which has to be
maintained by the user.
TL;DR: ContextManagers are a buttload of pitfalls. use() is probably
better, with much less inherent problems. And with the remaining
problems of the proposal being actually solvable.
Thanks,
Bob
