Skip to content

feat: test-only guard against nested same-pool connection acquisition - #411

Draft
tamassoltesz wants to merge 3 commits into
9.9from
fix/nested-conn-acquisition-guard
Draft

tamassoltesz wants to merge 3 commits into
9.9from
fix/nested-conn-acquisition-guard

Conversation

@tamassoltesz

Copy link
Copy Markdown
Collaborator

Why

A call chain that holds one pooled connection (inside a startTransaction) and then borrows a second from the same pool causes hold-and-wait pool exhaustion — the deadlock class behind the OAuth non-rotating-refresh regression and WebAuthN.updateUserEmail. Both reached the second connection through helper indirection, which a static (ArchUnit) rule can't catch without flooding false positives. A runtime tripwire at the point of acquisition is precise instead.

What

Track, per thread and per pool, that a startTransaction is in progress — marked in startTransactionHelper after the transaction takes its own connection, so its own borrow isn't flagged — and reject a new same-pool borrow while that mark is set (ConnectionPool.getConnection). It catches nested queries and nested startTransactions through any depth of helper indirection.

  • Keyed per pool, so a cross-tenant borrow on a different pool (e.g. updateLastActive) is allowed.
  • BulkImportProxyStorage is naturally exempt — it reuses its transaction connection and never reaches getNewConnection.
  • Active only under Start.isTesting — a no-op in production (zero overhead).

Pairing

Companion PR in supertokens-core (→ 12.3) mirrors the guard in the in-memory ConnectionPool and adds the regression test. Draft: full-suite CI is the authoritative run — the guard is a tripwire and may surface additional un-audited instances (that's it working); known ones are OAuth (being fixed) and WebAuthn (fixed). Verified locally: full clean multi-repo compile passes.

🤖 Generated with Claude Code

https://claude.ai/code/session_013dEv4WZTfLTkphYkQXTqPj

tamassoltesz and others added 3 commits September 28, 2026 10:12
A call chain that holds one pooled connection (inside a startTransaction) and
then borrows a SECOND from the same pool causes hold-and-wait pool exhaustion —
the deadlock class behind the OAuth non-rotating-refresh regression and the
WebAuthN.updateUserEmail issue. Both reached the second connection through helper
indirection, which static analysis (ArchUnit) can't catch without flooding false
positives; a runtime tripwire at the point of acquisition is precise instead.

Track, per thread and per pool, that a startTransaction is in progress (marked
after the transaction takes its own connection, so its own borrow isn't flagged),
and reject a new same-pool borrow while that mark is set. Keyed per pool, so a
cross-tenant borrow (different pool) is allowed; BulkImportProxyStorage is exempt
(it reuses its transaction connection, never reaching getNewConnection). Active
only under Start.isTesting — a no-op in production.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013dEv4WZTfLTkphYkQXTqPj
Mirror of the core change: warn (stderr, with the borrowing call site) by default
so the suite stays green while the pre-existing nested-acquisition instances are
cleaned up (PLAN-018); an opt-in throwOnNestedAcquisition flag fails fast.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013dEv4WZTfLTkphYkQXTqPj
@tamassoltesz

Copy link
Copy Markdown
Collaborator Author

Updated: the guard now warns by default (opt-in throw), mirroring the core change (PR #1461) — PLAN-018 Unit 1. Merges green now; default flips to throw after the cleanup.

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