Skip to content

THRIFT-1509: Retry interrupted stream_select in PHP TSocket - #4161

Open
slachiewicz wants to merge 1 commit into
apache:masterfrom
slachiewicz:THRIFT-1509
Open

slachiewicz wants to merge 1 commit into
apache:masterfrom
slachiewicz:THRIFT-1509

Conversation

@slachiewicz

Copy link
Copy Markdown
Member

Jira: THRIFT-1509

A signal arriving during stream_select makes it return false with EINTR, which TSocket::read() and write() reported as a fatal transport error. Both now retry when the suppressed warning names the interrupted system call ([4], or 10004 on Windows). The remaining time is recomputed from hrtime(), so interrupts do not restart the configured timeout; any other false still throws.

The new tests use pcntl_alarm and are marked #[RequiresFunction('pcntl_alarm')], so this also adds pcntl to the lib-php job's PHP extensions; without it they would be skipped there.

Verified: vendor/bin/phpunit -c lib/php/phpunit.xml lib/php/test/Unit/Lib/Transport/TSocketTest.php → the 3 new tests fail on master and pass here (34 tests); phpcs and phpstan (lib/php/phpstan.neon) clean; actionlint clean.

  • Did you create an Apache Jira ticket?
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-1509: describe my issue"?
  • Did you squash your changes to a single commit?
  • Did you do your best to avoid breaking changes? Only interrupted selects change behavior; timeouts and other errors are unchanged.

Client: php

A signal during stream_select makes it return false with errno EINTR,
which read() and write() reported as a fatal transport error. They now
retry while the suppressed warning says "[4]" / "Interrupted system call"
(or 10004 on Windows). The remaining time is recomputed from hrtime(), so
interrupts do not restart the configured timeout. Any other false still
throws. Tested with pcntl_alarm on a stream_socket_pair; the lib-php job
now enables pcntl so these tests run in CI instead of being skipped.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@mergeable mergeable Bot added php github_actions Pull requests that update GitHub Actions code labels Nov 8, 2026
@slachiewicz
slachiewicz marked this pull request as ready for review November 8, 2026 20:16
Copilot AI balanced review requested due to automatic review settings November 8, 2026 20:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update GitHub Actions code php

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants