On Wed, 5 Nov 2025 at 23:39, Bob Weinand <[email protected]> wrote:
>
> 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
>
Hi Bob,
I agree with your points, especially this one:
> It makes it very easy to not call the exitContext() method when calling
> enterContext() manually. [...] Thus, we have a big pitfall...
This is the exact reason why, in the `use()` thread, I suggested a
`Disposable` interface should **not** have an `enter()` method. The
language should just guarantee a single `dispose()` method is called
when the scope is exited (successfully or not). This completely avoids
the "unbalanced call" pitfall. SA tools can warn/error when
`dispose()` is called manually. API authors should be aware that the
resource might receive multiple `dispose()` calls and handle this
gracefully.
-----
Regarding your other valid concerns (nested calls, using objects after
the block, "leaking" the variable), I believe those are all solvable
by how the `use()` construct evolves, which is why I prefer its
foundation.
You noted:
> 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.
You're right, but the `use()` proposal has a clear path to solve this,
which is far superior IMO to the manual checks required by the
`ContextManager` design. As discussed in the `use()` thread, we could
later introduce:
1. A **`Resource`** marker interface: This would tell the engine to
handle the object specially (e.g., use weak references in backtraces
to prevent accidental refcount inflation).
2. A **`Disposable`** interface: This would have the `dispose()`
logic and could (and probably should) also be a `Resource`.
Furthermore, a `Disposable` interface opens up the possibility of
being supported *outside* the `use()` construct entirely. The engine
could guarantee `dispose()` is called whenever the object goes out of
*any* scope, just like a destructor but with crucial exception
awareness:
```php
function x() {
$disposable = new SomeDisposable();
return; // $disposable->dispose(null) called
}
function y() {
$disposable = new SomeDisposable();
throw new Error(); // $disposable->dispose($error) called
}
```
This would make `Disposable` a truly powerful, general-purpose RAII
mechanism, not just a feature tied to `use()`.
With these (future) additions, the `use()` construct could be enhanced
to **enforce a no-escape policy** specifically for `Resource`s. If
`use($r = get_resource())` finishes and `$r` still has references, the
engine could throw an error.
This combination would *programmatically prevent* the "use after free"
pitfall you described, rather than relying on manual checks inside
every single method.
All of those powerful safety checks can be added in the future. I
don't see why we need to cram them into the initial `use()` RFC. Its
current `__destruct`-based implementation is **already useful today**
for APIs designed to leverage RAII (like the Psl lock example). It's
normal that few APIs are designed this way now; the feature doesn't
exist yet. Psl just happens to support it because of its Hack origins.
This seems like a much safer and more extensible path.
