Make parser callbacks cancelable

[lalinsky]

Changed processPong() and processPing() to use cancelable locks instead of lockUncancelable().

These callbacks are invoked from the reader loop and should properly propagate cancellation for clean shutdown.

Changes

• processPong(): Changed from lockUncancelable() to try lock()
• processPing(): Changed from lockUncancelable() to try lock()

Impact

Allows proper cleanup when the reader fiber is canceled during connection shutdown.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR replaces uncancellable mutex lock acquisition with cancellable, error-checked locks in two handshake handler methods. processPong and processPing now use try self.mutex.lock(self.rt) instead of self.mutex.lockUncancelable(self.rt), allowing lock acquisition failures to propagate as errors rather than assuming success.

Changes

Cohort / File(s)	Summary
Lock acquisition strategy src/connection.zig	Changed processPong and processPing from uncancellable locks to cancellable, error-checked locks. Lock acquisition can now fail and return an error instead of proceeding unconditionally.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• More robust PING/PONG flush mechanism #38: Modifies the same processPong and processPing methods with refactoring to ping/pong counters and flush/reconnect logic, so any lock behavior changes here may interact with those modifications.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly describes the main change: making parser callbacks (processPing and processPong) cancelable by switching from uncancelable to cancelable locks.
Description check	✅ Passed	The description clearly explains what changed, why it matters for shutdown behavior, and lists the specific functions modified. It's directly related to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch parser-callbacks-cancelable

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/connection.zig`:
- Around line 1373-1375: Reader/flusher task cancellations are being logged as
errors because error.Canceled from mutex.lock(rt) in processPong/processPing
bubbles up to readerLoop/flusherLoop and is logged; update the error handling in
the readerLoop and flusherLoop error handlers to special-case error.Canceled (or
any err == error.Canceled) and either skip logging or log at debug level instead
of using log.err. Locate the readerLoop/flusherLoop error catch where it
currently calls log.err("Error in reader loop: {}", .{err}) and replace it with
a branch that checks for error.Canceled and no-ops or logs debug, otherwise
preserve the existing error logging; this avoids treating normal shutdown
cancellations as connection failures while leaving processPong/processPing and
mutex.lock unchanged.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, find where zio is defined/imported in nats.zig
rg -A 5 "zio\." src/connection.zig | head -30

Repository: lalinsky/nats.zig

Length of output: 1052

---

🏁 Script executed:

# Check build.zig to understand zio dependency
cat build.zig | head -100

Repository: lalinsky/nats.zig

Length of output: 4368

---

🏁 Script executed:

# Look for zio library in the repo or dependencies
find . -name "*.zig" -path "*/zio*" 2>/dev/null | head -20

Repository: lalinsky/nats.zig

Length of output: 43

---

🏁 Script executed:

# Search for Mutex lock implementation in the codebase
rg -A 10 "fn lock\(" --type zig | head -50

Repository: lalinsky/nats.zig

Length of output: 43

---

🏁 Script executed:

# Check build.zig.zon for zio dependency
cat build.zig.zon

Repository: lalinsky/nats.zig

Length of output: 2320

---

🏁 Script executed:

# Search for where processPong and processPing are called
rg "processPong|processPing" src/connection.zig -B 2 -A 2

Repository: lalinsky/nats.zig

Length of output: 325

---

🏁 Script executed:

# Look for error handling patterns around lock calls in the reader/parser
rg "try.*lock|catch.*lock" src/connection.zig -B 2 -A 2

Repository: lalinsky/nats.zig

Length of output: 2538

---

🏁 Script executed:

# Find where processPong and processPing are called
rg "processPong|processPing" src/ --type zig -B 3 -A 1

Repository: lalinsky/nats.zig

Length of output: 1330

---

🏁 Script executed:

# Find the reader task/main loop that processes messages
rg "reader|parseMessage" src/connection.zig -B 2 -A 3 | head -80

Repository: lalinsky/nats.zig

Length of output: 1147

---

🏁 Script executed:

# Look for the main dispatch/routing of protocol messages
rg "process.*\(" src/connection.zig | grep "pub fn\|\.process"

Repository: lalinsky/nats.zig

Length of output: 383

---

🏁 Script executed:

# Check if zio repo is cloned locally or cached
find . -name "zio" -type d 2>/dev/null | grep -v node_modules

Repository: lalinsky/nats.zig

Length of output: 43

---

🏁 Script executed:

# Look for error handling patterns specific to cancellation
rg "error\.Canceled|Canceled" src/ --type zig

Repository: lalinsky/nats.zig

Length of output: 652

---

🌐 Web query:

zio zig library Mutex.lock error set cancellation

💡 Result:

In lalinsky/zio (the Zig async I/O framework), Mutex.lock is a cancellation point: if the current task gets canceled while waiting to acquire the mutex, the lock operation can fail with error.Canceled. This is consistent with ZIO’s task-cancellation model where “operations in the canceled task will return error.Canceled.” [1]

So if you see a compile error like “expected void, found error{Canceled}!void” (or an error-set mismatch), the fix is to handle/propagate that error:

try mutex.lock(rt);
defer mutex.unlock(rt);

// critical section...

Or explicitly catch it:

mutex.lock(rt) catch |err| switch (err) {
    error.Canceled => return error.Canceled, // or handle it
    else => return err,
};
defer mutex.unlock(rt);

If you’re implementing higher-level sync primitives (e.g. Condition.wait) you sometimes must run cleanup that must not be interrupted; the project mentions a “shield” mode used by sync primitives so they can, for example, re-lock the mutex even if the wait was canceled, while still letting code afterward observe error.Canceled. [2]

Sources: [1] [2]

---

🏁 Script executed:

# Find and show reader loop error handling in detail
rg -A 10 "Error in reader loop" src/connection.zig

Repository: lalinsky/nats.zig

Length of output: 422

---

🏁 Script executed:

# Check if there's error.Canceled handling in reader loop or manager
rg -B 5 -A 5 "readerLoop|reader_task" src/connection.zig | head -100

Repository: lalinsky/nats.zig

Length of output: 1132

---

Filter error.Canceled from error logs during shutdown.

When the reader/flusher tasks are canceled during shutdown, any error.Canceled returned from mutex.lock(rt) in processPing and processPong will propagate back through the parser to the readerLoop. The error is currently caught and logged as log.err("Error in reader loop: {}", .{err}), which makes normal task cancellation appear as a connection failure.

Handle error.Canceled separately in the reader/flusher error handlers (around line 580) to either skip logging or log at debug level—it's expected during shutdown, not an actual error condition.

Also applies to: processPing (1399-1401)

🤖 Prompt for AI Agents

In `@src/connection.zig` around lines 1373 - 1375, Reader/flusher task
cancellations are being logged as errors because error.Canceled from
mutex.lock(rt) in processPong/processPing bubbles up to readerLoop/flusherLoop
and is logged; update the error handling in the readerLoop and flusherLoop error
handlers to special-case error.Canceled (or any err == error.Canceled) and
either skip logging or log at debug level instead of using log.err. Locate the
readerLoop/flusherLoop error catch where it currently calls log.err("Error in
reader loop: {}", .{err}) and replace it with a branch that checks for
error.Canceled and no-ops or logs debug, otherwise preserve the existing error
logging; this avoids treating normal shutdown cancellations as connection
failures while leaving processPong/processPing and mutex.lock unchanged.