Skip to content

[IoPoll] Take Time\Duration in Context::wait() - #645

Merged
nicolas-grekas merged 1 commit into
1.xfrom
io-poll-duration
Aug 7, 2026
Merged

[IoPoll] Take Time\Duration in Context::wait()#645
nicolas-grekas merged 1 commit into
1.xfrom
io-poll-duration

Conversation

@nicolas-grekas

Copy link
Copy Markdown
Member
Q A
Bug fix? no
New feature? no
Deprecations? no
Issues -
License MIT

Depends on #642, on top of which this branch sits — only the last commit belongs here.

php/php-src#23092 is merged and ships in 8.6 Beta 1, alongside Time\Duration. It replaces the $timeoutSeconds/$timeoutMicroseconds arguments of Io\Poll\Context::wait() by a single ?Time\Duration $timeout:

-public function wait(?int $timeoutSeconds = null, int $timeoutMicroseconds = 0, ?int $maxEvents = null): array
+public function wait(?\Time\Duration $timeout = null, ?int $maxEvents = null): array

The vendored phpt files get exactly the edits php-src made in that commit: wait(0) becomes wait(Time\Duration::fromSeconds(0)) and wait(0, 100000) becomes wait(Time\Duration::fromMicroseconds(100000)). Every file that was a byte-identical copy of ext/standard/tests/poll/ still is; the three that already differed (poll.inc, poll_stream_sock_rw_close.phpt, poll_stream_sock_rw_multi_level.phpt, all stale rather than adapted) keep exactly the differences they had.

symfony/polyfill-time is deliberately not a dependency: the polyfill never builds a Duration, it only reads ->negative, ->seconds and ->nanoseconds off the instance it is handed, and the type declaration is only resolved when a non-null value is passed. Calling wait() or wait(null) therefore works without it, and a caller that has a Duration to pass necessarily has the class already. The component README says where to get it.

One divergence worth a decision: native reports the negative-timeout error as argument #2,

if (timeout->duration.negative) {
    zend_argument_value_error(2, "must not be negative");

which looks like a leftover from the old signature, since $timeout is argument #1 and #2 is now $maxEvents. No phpt covers it. The polyfill reports #1; if upstream keeps #2 I will align.

@stof

stof commented Aug 7, 2026

Copy link
Copy Markdown
Member

which looks like a leftover from the old signature, since $timeout is argument #1 and #2 is now $maxEvents. No phpt covers it. The polyfill reports #1; if upstream keeps #2 I will align.

I suggest reporting an issue upstream about that.

@stof

stof commented Aug 7, 2026

Copy link
Copy Markdown
Member

the three that already differed (poll.inc, poll_stream_sock_rw_close.phpt, poll_stream_sock_rw_multi_level.phpt, all stale rather than adapted) keep exactly the differences they had.

do they differ intentionally or because php-src updated them after we vendored them ? In the later case, maybe we should sync them.

@TimWolla

TimWolla commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

I suggest reporting an issue upstream about that.

php/php-src#23099

@nicolas-grekas

Copy link
Copy Markdown
Member Author

php/php-src#23099 settles the argument number question: the negative timeout is reported as argument #1, which is what this polyfill already did, so there is nothing to align. Its new poll_ctx_wait.phpt is now vendored here byte-identical and passes as is, both messages included.

@nicolas-grekas
nicolas-grekas merged commit 0256d86 into 1.x Aug 7, 2026
20 checks passed
@nicolas-grekas
nicolas-grekas deleted the io-poll-duration branch August 7, 2026 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants