Skip to content

[PHP] Support non-blocking read streams #2339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 35 commits into from
Jul 13, 2025

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Jul 8, 2025

Description

This PR adds supports for reading from non-blocking streams
(stream_set_blocking($fp, true)) by aligning the streaming logic with
unix kernel:

  • stream_set_blocking() is now able to correctly mark the OS stream as
    non blocking via fcntl()
  • fd_read() either blocks or immediately returns based on stream type
  • js_open_process(), proc_open() et al. now work directly with
    kernel pipes. There's no more special event-based handling, polling, or
    special casing for pumping stdin data to the process.
  • PIPEFS no longer returns EWOULDBLOCK when the pipe is blocking, lacks
    data, and the other side is already closed
  • Removed special cases for polling processes, including
    PHPWASM.child_proc_by_fd. We rely on kernel pipes now.
  • Removed PHP 7.4 patch to treat blocking streams in a special way

Motivation for this patch

With this PR, we can use the Symfony Process component. It spawns a new
process via proc_open(), marks its output stream as non-blocking, and
immediately reads the initial output bytes if any are present. Before
this change, Playground would block until the first output is produced,
and, sometimes until the entire process finishes.

This PR is a pre-requisite to #2281

Other changes

This PR implements usleep() (via -Wl,--wrap=usleep). It turns out
that it wasn't actually blocking the execution and a few unit tests
relying on it only passed due to a buggy implementation of blocking
streams.

Follow-up work

  • Replace read() globally with something equivalent to wasm_read().
    Right now we only do this RUN /root/replace.sh 's/ret = read/ret = wasm_read/g' /root/php-src/main/streams/plain_wrapper.c
  • Consider not overriding php_pollfd_for and removing
    wasm_poll_socket() – wrapping select/poll/read syscalls may be enough
  • We could remove more special casing, including socket polling inside
    wasm_poll_socket, the js_popen_to_file function, awaitData et al.

Testing Instructions (or ideally a Blueprint)

Tests are included so just confirm the CI is green.

@adamziel adamziel changed the title Support non-blocking read streams [PHP] Support non-blocking read streams Jul 8, 2025
@adamziel
Copy link
Collaborator Author

adamziel commented Jul 9, 2025

It seems like we could just remove our php_pollfd_for override and everything still works. I'll leave that out so to enable finally merging this PR, but that would make a decent follow-up PR.

@adamziel adamziel merged commit 1f2730b into trunk Jul 13, 2025
24 of 25 checks passed
@adamziel adamziel deleted the partial-nonblocking-streams-support branch July 13, 2025 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant