Skip to content

fix(network): dedicated URLSession for BTW streaming (LUM-820, LUM-903)#27250

Merged
vex-assistant-bot[bot] merged 1 commit into
mainfrom
do/lum-820-stream-session
Apr 21, 2026
Merged

fix(network): dedicated URLSession for BTW streaming (LUM-820, LUM-903)#27250
vex-assistant-bot[bot] merged 1 commit into
mainfrom
do/lum-820-stream-session

Conversation

@ashleeradka

@ashleeradka ashleeradka commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes EXC_BAD_ACCESS crashes in BTW streaming (Sentry MACOS-EB, 8 events / 4 users) by adopting the PR fix: use dedicated URLSession per SSE connection to prevent EXC_BAD_ACCESS #25396 per-call URLSession pattern for streamPost and streamPostWithRetry.
  • BtwClient now creates a dedicated URLSession per sendMessage call and invalidates it when the stream finishes (including on error or consumer cancellation).
  • GatewayHTTPClient.streamPost and streamPostWithRetry gain a session: URLSession = .shared parameter — default-valued so existing callers are unaffected.

Why

Running streaming URLSession.bytes(for:) on URLSession.shared races Task.cancel() against the AsyncBytes cooperative-pool iterator. When the shared session's internal state is torn down while the iterator is still reading, CheckedContinuation.resume hits freed memory and the process crashes with KERN_INVALID_ADDRESS at 0x28 in NSURLSession.data__NSCFURLSessionDelegateWrapper_task_onqueue_didFinish.

The fix in #25396 (EventStreamClient SSE) moved the main event stream to a per-connection session so invalidateAndCancel() can tear down the data task on its own terms. This PR ports the same pattern to the remaining streaming paths (streamPost, streamPostWithRetry) and to their sole caller, BtwClient.

Tickets resolved

  • LUM-820 — EXC_BAD_ACCESS in CheckedContinuation.resume during URLSession.data callback
  • LUM-903 — streamPost/streamPostWithRetry still use URLSession.shared

Deferred to LUM-1001

A separate Devin session is landing SafeAsyncBytes — a delegate-based async bytes wrapper that survives concurrent session invalidation. That's a larger change; this PR intentionally stops at the minimal #25396 pattern to unblock users on the crash signal now. The two are compatible: the session parameter added here carries through.

Testing

  • Exercise BTW side-chain (inline "btw …" question) in a conversation; confirm streamed response renders normally.
  • Cancel a BTW stream mid-flight (navigate away or start a new BTW while one is in flight); confirm no crash.
  • Trigger a 401 during BTW (e.g. force a token expiry); confirm the retry path succeeds on the same per-call session.

🤖 Generated with Claude Code


Open in Devin Review

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@ashleeradka ashleeradka self-assigned this Apr 21, 2026

@vex-assistant-bot vex-assistant-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE

Value: Fixes real EXC_BAD_ACCESS crashes affecting 4+ users (Sentry MACOS-EB) by eliminating the use-after-free race between Task.cancel() and the AsyncBytes iterator on URLSession.shared.

What this does: Ports the proven PR #25396 per-call URLSession pattern from EventStreamClient to the remaining streaming paths — streamPost and streamPostWithRetry in GatewayHTTPClient. BtwClient creates a dedicated session per sendMessage call and invalidates it via defer when streamBtw returns.

Verified:

  • ✅ Session lifecycle is correct — defer { session.invalidateAndCancel() } fires AFTER for try await line in bytes.lines completes in streamBtw, so bytes are fully consumed before teardown
  • ✅ Cancellation path is clean — continuation.onTermination cancels the Task → Task.isCancelled breaks the loop → function returns → defer fires
  • ✅ Retry path at line 549 correctly reuses the same dedicated session (not .shared)
  • ✅ Default parameter (session: URLSession = .shared) preserves backward compatibility for any future callers
  • BtwClient is the only external caller of streamPostWithRetry, and streamPost has no direct external callers — all vulnerable streaming paths are covered
  • ✅ Pattern matches EventStreamClient (PR #25396) exactly, just scoped to function lifetime instead of object lifetime (correct for BTW's request-response pattern vs SSE's long-lived connection)

Testing note: Exercise a BTW side-chain message (inline "btw" question) and verify the streamed response renders. Also test cancelling mid-stream (navigate away during a BTW response) to confirm clean teardown.

@vex-assistant-bot vex-assistant-bot Bot merged commit ae9a721 into main Apr 21, 2026
7 checks passed
@vex-assistant-bot vex-assistant-bot Bot deleted the do/lum-820-stream-session branch April 21, 2026 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant