Repository navigation
Conversation
Add tests for two bugs: Bug mitchellh#209: tick(0) EV_DELETE flush - kqueue tick(0) may leave stale kernel filters when disarming. Six tests added covering multiple disarms, write/read disarm on same fd, rapid cycles, no-op tick, stale filter detection, and the kevent-event-processing race path (fires without fix, passes with fix). Bug mitchellh#170: rearm from non-kqueue completions - when a completion that completes without kqueue registration returns .rearm, it is pushed to submissions with .dead state which gets routed to stop_completion instead of start. Integration test added (fails without fix, passes with fix). Queue unit tests added verifying re-enqueue across queues, drain-and-refill, and rapid cycle invariants.
…xt in submit Bug mitchellh#209: kqueue tick(0) EV_DELETE flush - Changed early exit condition from 'if (wait == 0) break' to 'if (wait == 0 and changes == 0) break' so pending EV_DELETE kevent changes from the event-processing loop are flushed before tick(0) returns. Bug mitchellh#170: rearm state correction - When a non-kqueue completion (close, shutdown, threadpool fallback) returns .rearm, set flags.state to .adding before pushing to the submissions queue. Previously .dead state caused submit() to route through stop_completion (cancel) instead of start() (resubmit). Issue mitchellh#169: defensive next-pointer clear in submit() - Explicitly clear c.next before push to completions queue when processing kevent-returned events in submit(). This prevents an assertion failure if a completion is present in multiple queues concurrently (race condition).
|
Facing this issue almost everyday, can we prioritize this fix ? |
|
@mitchellh I am sorry if this is poor form, but is there anything I can do to move this PR forward? |
|
The Baseline Candidate -if (wait == 0) break;
+if (wait == 0 and changes == 0) break;Run The stale level-triggered EOF re-enters the kevent event path. That path unconditionally decrements The changed condition submits the staged This result validates the deletion-flush hunk only. It does not validate the other two hunks or a separate kqueue EBADF probe. |
|
Follow-up: the deletion-flush hunk also fixes both ztls write-cancellation variants on kqueue. With only that hunk, macOS run
Without the hunk, isolated runs produce an integer-overflow panic in each write test. The runner reports only the panic address, so this result does not prove the exact source line. ztls now pins this PR at |
|
I've also created a PR that touches |
…xt in submit Bug mitchellh#209: kqueue tick(0) EV_DELETE flush - Changed early exit condition from 'if (wait == 0) break' to 'if (wait == 0 and changes == 0) break' so pending EV_DELETE kevent changes from the event-processing loop are flushed before tick(0) returns. Bug mitchellh#170: rearm state correction - When a non-kqueue completion (close, shutdown, threadpool fallback) returns .rearm, set flags.state to .adding before pushing to the submissions queue. Previously .dead state caused submit() to route through stop_completion (cancel) instead of start() (resubmit). Issue mitchellh#169: defensive next-pointer clear in submit() - Explicitly clear c.next before push to completions queue when processing kevent-returned events in submit(). This prevents an assertion failure if a completion is present in multiple queues concurrently (race condition). (cherry picked from mitchellh#224; the .rearm arm is merged with this fork's keep-active fix: a completion still registered with kqueue stays .active, one never registered is resubmitted as .adding) (cherry picked from commit a7502d6) (cherry picked from commit 570e3b0)
…xt in submit Bug mitchellh#209: kqueue tick(0) EV_DELETE flush - Changed early exit condition from 'if (wait == 0) break' to 'if (wait == 0 and changes == 0) break' so pending EV_DELETE kevent changes from the event-processing loop are flushed before tick(0) returns. Bug mitchellh#170: rearm state correction - When a non-kqueue completion (close, shutdown, threadpool fallback) returns .rearm, set flags.state to .adding before pushing to the submissions queue. Previously .dead state caused submit() to route through stop_completion (cancel) instead of start() (resubmit). Issue mitchellh#169: defensive next-pointer clear in submit() - Explicitly clear c.next before push to completions queue when processing kevent-returned events in submit(). This prevents an assertion failure if a completion is present in multiple queues concurrently (race condition). (cherry picked from mitchellh#224; the .rearm arm is merged with this fork's keep-active fix: a completion still registered with kqueue stays .active, one never registered is resubmitted as .adding) (cherry picked from commit a7502d6) (cherry picked from commit 570e3b0)
Zig 0.17 removes array multiplication. @Splat builds the same array on 0.16 too. Assisted-by: Claude Opus 5.5
…xt in submit Bug mitchellh#209: kqueue tick(0) EV_DELETE flush - Changed early exit condition from 'if (wait == 0) break' to 'if (wait == 0 and changes == 0) break' so pending EV_DELETE kevent changes from the event-processing loop are flushed before tick(0) returns. Bug mitchellh#170: rearm state correction - When a non-kqueue completion (close, shutdown, threadpool fallback) returns .rearm, set flags.state to .adding before pushing to the submissions queue. Previously .dead state caused submit() to route through stop_completion (cancel) instead of start() (resubmit). Issue mitchellh#169: defensive next-pointer clear in submit() - Explicitly clear c.next before push to completions queue when processing kevent-returned events in submit(). This prevents an assertion failure if a completion is present in multiple queues concurrently (race condition). (cherry picked from mitchellh#224; the .rearm arm is merged with this fork's keep-active fix: a completion still registered with kqueue stays .active, one never registered is resubmitted as .adding) (cherry picked from commit a7502d6) (cherry picked from commit 570e3b0)
…xt in submit Bug mitchellh#209: kqueue tick(0) EV_DELETE flush - Changed early exit condition from 'if (wait == 0) break' to 'if (wait == 0 and changes == 0) break' so pending EV_DELETE kevent changes from the event-processing loop are flushed before tick(0) returns. Bug mitchellh#170: rearm state correction - When a non-kqueue completion (close, shutdown, threadpool fallback) returns .rearm, set flags.state to .adding before pushing to the submissions queue. Previously .dead state caused submit() to route through stop_completion (cancel) instead of start() (resubmit). Issue mitchellh#169: defensive next-pointer clear in submit() - Explicitly clear c.next before push to completions queue when processing kevent-returned events in submit(). This prevents an assertion failure if a completion is present in multiple queues concurrently (race condition). (cherry picked from mitchellh#224; the .rearm arm is merged with this fork's keep-active fix: a completion still registered with kqueue stays .active, one never registered is resubmitted as .adding) (cherry picked from commit a7502d6) (cherry picked from commit 570e3b0)
…xt in submit Bug mitchellh#209: kqueue tick(0) EV_DELETE flush - Changed early exit condition from 'if (wait == 0) break' to 'if (wait == 0 and changes == 0) break' so pending EV_DELETE kevent changes from the event-processing loop are flushed before tick(0) returns. Bug mitchellh#170: rearm state correction - When a non-kqueue completion (close, shutdown, threadpool fallback) returns .rearm, set flags.state to .adding before pushing to the submissions queue. Previously .dead state caused submit() to route through stop_completion (cancel) instead of start() (resubmit). Issue mitchellh#169: defensive next-pointer clear in submit() - Explicitly clear c.next before push to completions queue when processing kevent-returned events in submit(). This prevents an assertion failure if a completion is present in multiple queues concurrently (race condition). (cherry picked from mitchellh#224; the .rearm arm is merged with this fork's keep-active fix: a completion still registered with kqueue stays .active, one never registered is resubmitted as .adding) (cherry picked from commit a7502d6) (cherry picked from commit 570e3b0)
XEV-003 and XEV-004 cover the submit-reported rearm and duplicate event paths, but nested submits from callbacks can still requeue the completion currently being dispatched or a completion from the outer tick batch. The no_wait path also dropped staged EV_DELETE changes, and EV_ERROR and non-kqueue rearm paths left the active count unbalanced. Guard both dispatch loops by state, threadpool mode, and completion-queue membership; flush pending deletes before a no_wait tick returns; balance EV_ERROR; and resubmit non-kqueue rearm completions in `.adding` state. Queued-result cancellation remains ordered after the callback because replacing an already-consumed accept or read result can lose a socket or data; that pre-existing limitation is documented in stop_completion. Ledger: XEV-005 Upstream: None (downstream; related to mitchellh#169, mitchellh#122, and mitchellh#224) Invariant: A completion is performed and called back at most once per dispatch, staged deletes reach kqueue before no_wait returns, and every EV_ERROR or non-kqueue rearm keeps the active count balanced. Test evidence: "kqueue: nested submit does not requeue the current completion", "kqueue: nested submit does not double-dispatch an outer batch", "kqueue: EV_ERROR balances the active count", and "kqueue: rearm from a non-kqueue completion resubmits". Retirement criterion: the fork base contains equivalent dispatch and accounting guards.
XEV-003 and XEV-004 cover the submit-reported rearm and duplicate event paths, but nested submits from callbacks can still requeue the completion currently being dispatched or a completion from the outer tick batch. The no_wait path also dropped staged EV_DELETE changes, and EV_ERROR and non-kqueue rearm paths left the active count unbalanced. Guard both dispatch loops by state, threadpool mode, and completion-queue membership; flush pending deletes before a no_wait tick returns; balance EV_ERROR; and resubmit non-kqueue rearm completions in `.adding` state. Queued-result cancellation remains ordered after the callback because replacing an already-consumed accept or read result can lose a socket or data; that pre-existing limitation is documented in stop_completion. Ledger: XEV-005 Upstream: None (downstream; related to mitchellh#169, mitchellh#122, and mitchellh#224) Invariant: A completion is performed and called back at most once per dispatch, staged deletes reach kqueue before no_wait returns, and every EV_ERROR or non-kqueue rearm keeps the active count balanced. Test evidence: "kqueue: nested submit does not requeue the current completion", "kqueue: nested submit does not double-dispatch an outer batch", "kqueue: EV_ERROR balances the active count", and "kqueue: rearm from a non-kqueue completion resubmits". Retirement criterion: the fork base contains equivalent dispatch and accounting guards.
…xt in submit Bug mitchellh#209: kqueue tick(0) EV_DELETE flush - Changed early exit condition from 'if (wait == 0) break' to 'if (wait == 0 and changes == 0) break' so pending EV_DELETE kevent changes from the event-processing loop are flushed before tick(0) returns. Bug mitchellh#170: rearm state correction - When a non-kqueue completion (close, shutdown, threadpool fallback) returns .rearm, set flags.state to .adding before pushing to the submissions queue. Previously .dead state caused submit() to route through stop_completion (cancel) instead of start() (resubmit). Issue mitchellh#169: defensive next-pointer clear in submit() - Explicitly clear c.next before push to completions queue when processing kevent-returned events in submit(). This prevents an assertion failure if a completion is present in multiple queues concurrently (race condition). (cherry picked from mitchellh#224; the .rearm arm is merged with this fork's keep-active fix: a completion still registered with kqueue stays .active, one never registered is resubmitted as .adding) (cherry picked from commit a7502d6) (cherry picked from commit 570e3b0)
…xt in submit Bug mitchellh#209: kqueue tick(0) EV_DELETE flush - Changed early exit condition from 'if (wait == 0) break' to 'if (wait == 0 and changes == 0) break' so pending EV_DELETE kevent changes from the event-processing loop are flushed before tick(0) returns. Bug mitchellh#170: rearm state correction - When a non-kqueue completion (close, shutdown, threadpool fallback) returns .rearm, set flags.state to .adding before pushing to the submissions queue. Previously .dead state caused submit() to route through stop_completion (cancel) instead of start() (resubmit). Issue mitchellh#169: defensive next-pointer clear in submit() - Explicitly clear c.next before push to completions queue when processing kevent-returned events in submit(). This prevents an assertion failure if a completion is present in multiple queues concurrently (race condition). (cherry picked from mitchellh#224; the .rearm arm is merged with this fork's keep-active fix: a completion still registered with kqueue stays .active, one never registered is resubmitted as .adding) (cherry picked from commit a7502d6) (cherry picked from commit 570e3b0)
PR mitchellh#224 alone left invalid-state under c64 (uWebZockets 12/28). Keep submit-reported rearms .active, skip duplicate level-triggered queueing before perform, guard nested submit, and balance EV_ERROR/non-kqueue rearm — matching farbenbuilds/libxev c49f6df with regression tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Three fixes for the kqueue backend, each with a reproducing test:
tick(0)now flushes pending EV_DELETE changes before returning. When a disarm occurs in the event-processing path, the EV_DELETE was queued buttick(0)exited before flushing it to the kernel, leaving a stale filter with a dangling udata pointer. Related to PR fix th .no_wait callback called multi times issue. #209.Completions that return
.rearmwithout kqueue registration (close, shutdown, no-threadpool fallback) now set state to.addingbefore re-entering the submissions queue. Previously the stale.deadstate causedsubmit()to route throughstop_completion(cancellation) instead ofstart()(resubmit). Related to PR queue: clear next before re-enqueue #170.Defensive
c.next = nullbefore pushing kevent-returned completions to the completions queue insubmit(). Prevents the queue.zig assertion failure when a completion is present in multiple queues concurrently. Fixes issue [bug] v.next is not proper cleared for re-enqueue a completion between different queues #169.10 tests added (all fail without their respective fixes):