Conversation
A StreamPollHandle keeps a reference to the stream resource, but an explicit fclose() still closes the stream underneath the watcher. The watcher could then no longer resolve its fd, so its backend registration was left behind: a recycled fd number confused the registry and the poll backend, and on epoll a duplicated fd could keep the interest alive and hand a freed watcher back from wait(). Streams now keep a list of their watchers and notify them from php_stream_free() before the fd is closed, so every watcher is unregistered while the fd is still valid. The Context registry is keyed by the registered fd, which the watcher caches, so remove() no longer depends on the stream and an fd number reused after a close is detected on add(). A watcher retired by the close reports inactive and its remove() becomes a no-op. modifyEvents() re-adds a fired one-shot registration that the backend dropped, and the kqueue backend drops its tracking entry when there is nothing left to delete. This follows the approach of phpGH-22848 by Ilia Alshanetsky.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A StreamPollHandle keeps a reference to the stream resource, but an explicit fclose() still closes the stream underneath the watcher. The watcher could then no longer resolve its fd, so its backend registration was left behind: a recycled fd number confused the registry and the poll backend, and on epoll a duplicated fd could keep the interest alive and hand a freed watcher back from wait().
Streams now keep a list of their watchers and notify them from php_stream_free() before the fd is closed, so every watcher is unregistered while the fd is still valid. The Context registry is keyed by the registered fd, which the watcher caches, so remove() no longer depends on the stream and an fd number reused after a close is detected on add(). A watcher retired by the close reports inactive and its remove() becomes a no-op. modifyEvents() re-adds a fired one-shot registration that the backend dropped, and the kqueue backend drops its tracking entry when there is nothing left to delete.
I tried couple of approaches but ended up following the approach from GH-22848 by @iliaal. It differs a bit and fixes few more things but the main idea is from there.