Repository navigation
Conversation
With handle_cross_request_promise_resolution, a promise settled from another request is settled in the request that created it, and the settlement is dropped if that request's IoContext is gone. So once the creating IoContext is destroyed, a promise that is still pending never settles. A request awaiting such a promise waited on a cross-thread fulfiller that only the promise's settlement could fulfill. awaitIo() kept it from being detected as hung, so the request never finished. That happened when the creating request was canceled as hung with the settlement still queued, and when a request settled and then awaited a promise whose creator had already ended. The waiter now also wakes when the creating IoContext is destroyed, via a new DeleteQueue::whenDefunct(). If the promise is still pending then, the await rejects with an error that says why. Without the flag, another request can still settle the promise after its creator ends, so the waiter keeps waiting as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Superseded by the review of
2dc4e796.
This review, of b839de19
✅ Approved — No findings. Waiters now wake when the creating IoContext is destroyed, which fixes the hang; the pending-state check rules out false failures, and the tests cover both cases.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No issues found.
📖 AGENTS.md: ✅ LGTM
No issues found.
📋 Code Quality: ✅ LGTM
No issues found.
⚡ Performance: ✅ LGTM
No issues found. Fulfillers of canceled waits are pruned once their count doubles, so long-lived contexts don't accumulate them.
📝 Documentation: ✅ LGTM
No issues found.
Reviewed commit: b839de19
Fixes -Werror=unused-value from discarding a nodiscard Promise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ Approved — No findings. The only change since the last review is a test-only fix for an unused-value warning, and the hang fix itself is still correct.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No security concerns.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md updates needed.
📋 Code Quality: ✅ LGTM
The updated whenTargetDestroyed() test in io-own-test.c++ checks four things:
- The wait stays pending while the context is alive.
- Pruning runs across the canceled waits.
- The wait resolves when the context is destroyed.
- A wait registered after destruction resolves immediately.
The destructor moves the fulfillers out under the lock and fulfills them after releasing it. That matches how it already handles the existing cross-thread fulfiller.
⚡ Performance: ✅ LGTM
Fulfillers of canceled waits are pruned once their count doubles, so the cost per call stays constant on average. The extra join in addCrossThreadPromiseWaiter runs only when the flag is on and adds very little overhead.
📝 Documentation: ✅ LGTM
No documentation issues.
Reviewed commit: 2dc4e796
With
handle_cross_request_promise_resolution, a promise that another request settles is settled in the request that created it. If that request's IoContext is gone, the settlement is dropped. So once the creating IoContext is destroyed, a promise that is still pending can never settle.A request awaiting such a promise (through the
PromiseCrossContextCallbackandaddCrossThreadPromiseWaiter()) waited on a cross-thread fulfiller that only the promise's settlement could fulfill.awaitIo()kept the hang detector from seeing it, so the request never finished. Two cases hit this:IoContext dropped queued cross-context actions when destroyed.The waiter now also wakes when the creating IoContext is destroyed, through a new
DeleteQueue::whenDefunct()(exposed asIoCrossContextExecutor::whenTargetDestroyed()). If the promise is still pending at that point, the await rejects with an error that explains why. If it has settled, the await goes on as before, so losing the race with a real settlement can't cause a spurious failure.Without the flag, another request can still settle the promise after its creator ends, so the waiter keeps waiting as before.
The fulfillers of canceled waits are pruned once their count doubles, so a long-lived context such as an actor doesn't accumulate them.
Tests:
cross-context-promise-test: newawaitingOrphanedPromiseFailscovers both cases. Without the fix it times out.io-own-test:whenTargetDestroyed()resolves on destruction, resolves immediately afterwards, and survives pruning.cross-request-promise-resolve.ew-test-bindisabled since 2024-10.🤖 Generated with Claude Code