[SLOP(claude-opus-4-8-medium)] fix(rivetkit-napi): reap completed tasks from event-loop JoinSet

[MasterPtato]

No description provided.

[MasterPtato]

Stack for rivet-dev/rivet

• [SLOP(claude-opus-4-8-medium)] fix(rivetkit-napi): reap completed tasks from event-loop JoinSet #5315 vtlztrkn · 2026-06-22 22:42:02 👈
• [SLOP(claude-opus-4-8-medium)] chore: add profiling cargo profile for heaptrack leak investigation #5314 xsoysnqs · 2026-06-22 22:42:01
• [SLOP(claude-opus-4-8)] fix(kitchen-sink): serve built frontend from server #5306 rymmurrk · 2026-06-19 01:04:14
• main

Get stack: forklift get 5315
Push local edits: forklift submit
Merge when ready: forklift merge 5315

[claude]

Code Review

This PR fixes a native RSS memory leak in the NAPI event loop: the JoinSet holding background task handles was never joined during normal actor operation, so every completed task (action handler, WebSocket open, disconnect callback, etc.) retained its allocation for the entire actor lifetime. The fix restructures run_event_loop from a single-branch while let into a tokio::select! that interleaves task reaping with event dispatch.

The fix is correct and well-motivated. One plausible concern and one convention note below.

---

PLAUSIBLE — Unbiased select! can transiently delay event processing under task burst load

rivetkit-typescript/packages/rivetkit-napi/src/napi_actor_events.rs line 187

tokio::select! without the biased attribute uses pseudo-random branch selection when multiple arms are simultaneously ready. If a large number of tasks complete in the same reactor tick while an event is queued in events.recv(), the tasks.join_next() arm can win the coin flip repeatedly before events.recv() is selected. The original while let Some(event) = events.recv().await loop was strictly event-ordered with no competition.

In practice the impact is bounded: each reap is sub-microsecond, the grace deadline is 15 s, and RunGracefulCleanup has its own shutdown_deadline_token fallback — so this cannot produce a correctness failure under current semantics. But adding biased and listing events.recv() first would restore the original priority and make the intent explicit:

tokio::select! {
    biased;
    event = events.recv() => { ... }
    Some(result) = tasks.join_next(), if !tasks.is_empty() => { ... }
}

---

Convention — New tracing::error! missing actor_id structured field

rivetkit-typescript/packages/rivetkit-napi/src/napi_actor_events.rs line 195

CLAUDE.md: "rivetkit-core runtime logs should include actor_id and stable structured fields."

The new error log is:

tracing::error!(?error, "napi background task failed to join");

ctx is in scope (run_event_loop receives ctx: &ActorContext). The equivalent log in drain_tasks (line 857) also omits it, but that's pre-existing. The new line should include actor_id:

tracing::error!(actor_id = %ctx.inner().actor_id(), ?error, "napi background task failed to join");

---

No other issues found. The is_empty() guard on the join_next branch is correct (prevents registering a waker on an empty set). The drain_tasks ordering (pump_registered_tasks before is_empty()) correctly handles chained task registration. Tasks exit the JoinSet cleanly via CancellationToken (not abort()), so the missing is_cancelled() guard in drain_tasks has no observable effect.

[railway-app]

🚅 Deployed to the rivet-pr-5315 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
kitchen-sink	😴 Sleeping (View Logs)	Web	Jun 22, 2026 at 10:56 pm
website	😴 Sleeping (View Logs)	Web	Jun 22, 2026 at 10:54 pm
frontend-cloud	😴 Sleeping (View Logs)	Web	Jun 22, 2026 at 10:53 pm
frontend-inspector	😴 Sleeping (View Logs)	Web	Jun 22, 2026 at 10:51 pm
ladle	✅ Success (View Logs)	Web	Jun 22, 2026 at 10:44 pm
mcp-hub	✅ Success (View Logs)	Web	Jun 22, 2026 at 10:43 pm