#141 Pool: wake parked waiter when broken-release destroys resource#142
Merged
Conversation
beforeRelease=false (e.g. PDO conn_broken after Toxiproxy reset_peer) destroyed the resource and decremented active_count but never woke a parked waiter. The slot was conceptually free but only release() wakes waiters — with every connection in flight failing, parked coroutines deadlocked until the global Async\DeadlockError detector fired. Fix in zend_async_pool_release: after destroying a broken resource, call pool_wake_waiter so a parked coroutine retries the factory (and either succeeds or propagates its own exception, cascading to the next waiter). Defensive fix in zend_async_pool_acquire: factory failure on the slot-reservation path now also wakes a waiter and throws PoolException when the factory returns false without setting an exception — fail-fast instead of falling through to pool_wait_for_resource with nothing able to wake it. Regression test: tests/pool/054-pool_broken_release_wakes_waiters.phpt (fails without fix, passes with).
Both LINUX_X64_DEBUG_ZTS and LINUX_X64_RELEASE_NTS on PR #142 failed at "Install build dependencies" with a 502 Bad Gateway from github.com while fetching curl-8.5.0.tar.gz. Pure infra flake — but the bare wget gave up on first 5xx with no retry. Add --tries=5 --waitretry=10 --retry-connrefused --retry-on-http-error=429,500,502,503,504 so the next transient GitHub hiccup doesn't tank an otherwise-green PR.
EdmondDantes
deleted the
141-pdo-pool-deadlocks-when-over-saturated-against-a-permanently-failing-connection-chaos-finding
branch
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.
Summary
Closes #141.
Root cause in
zend_async_pool_release: whenbeforeReleasereturned false (PDO marksconn_broken=trueafter a Toxiproxyreset_peer), the pool destroyed the resource and decrementedactive_countbut never woke a parked waiter. The slot was conceptually free but onlyrelease()wakes waiters — with every connection in flight failing, parked coroutines deadlocked until the globalAsync\DeadlockErrordetector fired.Fix
zend_async_pool_release(primary): after destroying a broken resource, callpool_wake_waiterso a parked coroutine retries the factory. If that factory also fails, the cascade continues — each waiter propagates its own exception cleanly.zend_async_pool_acquire(defensive): factory failure on the slot-reservation path now also wakes a waiter, and throwsPoolExceptionwhen the factory returns false without setting an exception — fail-fast instead of falling through topool_wait_for_resourcewith nothing able to wake it.Test plan
tests/pool/054-pool_broken_release_wakes_waiters.phpt(fails without fix, passes with — verified by reverting fix and re-running)ext/async/tests/pool/)max=1, 3 coroutines,reset_peer): all 3 coroutines get clean PDOException, noDeadlockError, no global deadlock detector firing. 5/5 stable runs.