Hi
Am 2026-02-22 17:16, schrieb Jakub Zelenka:
Thanks for the feedback!
Thank you. I've given the RFC another read now and have some minor
things to remark:
1. I don't think it makes sense for the majority of the classes to be
serialized, because they rely on “in-process” state, specifically file
descriptors. This includes Context, Watcher, and the handles.
Serializing the enum and Exceptions by themselves is fine. The RFC
should note that serialization is prevented and the implementation
adjusted.
2. Similarly for any new classes, I recommend to always set
`@strict-properties` to prevent shenanigans with folks accidentally
adding dynamic properties to these classes. This should be done for
everything, including the exceptions.
3. I have a question as someone who never used Fibers in production: Is
it necessary or helpful for the API to provide some “convenience”
functionality related to Fibers (e.g. automatically suspending or
unsuspending Fibers)? I'm asking now, because adding that functionality
later might be a breaking change in behavior. If it's not necessary or
can be added in a backwards compatible way, that's fine, of course.
On Sun, Jan 18, 2026 at 1:24 PM Tim Düsterhus <[email protected]> wrote:
- Context::wait() takes a millisecond timeout. This feels
insufficiently
granular. I am wondering if we should rather use either micro (as
returned by microtime) or nanoseconds (as returned by hrtime) - or
possible an instance of DateInterval (which would use microsecond
resolution).
Yeah this makes sense. I used the same convention that is used in
streams
which means two params so it is changed to:
public function wait(int $timeoutSeconds = -1, int $timeoutMicroseconds
=
0, int $maxEvents = -1): array {}
I feel that using two parameters is providing for a very clunky API,
which is also acknowledged by the doc comment on $timeoutMicroseconds
"Only used when $timeoutSeconds >= 0". I would have suggested
DateInterval again, but I'm just seeing that it seems there is no
documented way to manually construct a DateInterval with microseconds.
Perhaps the following suggestion would be a good middleground:
- Change $timeoutSeconds to int|null: If null is specified, the timeout
is infinite.
- Throw ValueError if $timeoutSeconds < 0
- Throw ValueError if $timeoutMicroseconds < 0
- Throw ValueError if $timeoutSeconds === null && timeoutMicroseconds >
0
`null` is a much more natural value for “no timeout” and by throwing
ValueError we make sure that we don't have a situation where a parameter
is silently ignored.
Similarly for `$maxEvents` it might make sense to use `null` for “use a
default value”? Also: What happens if I specify `$maxEvents = 0`? Is
this useful?
- For Handle I am wondering about abstract class vs interface. Since
the
Handle presumably doesn't have any non-abstract methods, an interface
feels more correct / more flexible.
I have been considering this already and there are reasons why I prefer
abstract method here.
The abstract method is better for internal implementation and limit
overhead of PHP calls that would be required for interface. The
advantage
is that I can use internal struct API through C operations which allows
bypasing the call and can do optimization for some backends - e.g.
timeouts
for kqueue don't need file descriptor.
You can bypass the “VM method” for interfaces and abstract methods
alike. A good example would be ext/random which allows changing the
engines by the Random\Engine interface, but for engines implemented in
C, the Randomizer doesn't go through the userland API, but instead calls
the engines directly.
I also think that there is not really much use case for user space to
implement their own handles so such interface would be used only
internally
anyway.
This applies equally to interfaces and abstract methods. The abstract
base class however will make it much weirder when a specific (future)
handle might need to implement additional interfaces or abstract
classes.
In addition interface would effectively expose the internal stream fd
which
is currently hidden and makes harder messing up with stream fd which
might
cause various issues.
I don't understand that point. For both an interface and an abstract
method, the method would exist on the child class and thus can be called
by a developer.
- For Handle: What happens if I extend this class in userland and
attach
it to a Context. Will things break? What if I return the number of a
non-existent FD?
Depending on backend it will result either in FailedHandleAddException
(epoll, kqueue, possible event ports as well) or on Windows it would
likely
fail during wait with FailedPollWaitException. So it can be handled by
catching FailedPollOperationException. This is however very unlikely
edge
case so I don't think there is much point to worry about consistency.
As I
said the use case for implementing custom Handle is a bit moot.
Okay, I just wanted to make sure that nothing *bad* happens. If it
safely errors, this is fine.
Best regards
Tim Düsterhus
