AdmissionControl permits leak when HTTP/2 stream processes terminate with their connection

Aperta
  • Qualcosa non va?
#4,772 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
35/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
elixir
Ambito
backend, networking

Direzione di ricerca

Inizia con Electric.Plug.ServeShapePlug.call/2 e l'archiviazione dei permessi di AdmissionControl e gli aggiornamenti del contatore ETS, quindi esamina il comportamento di terminazione degli stream HTTP/2 di Bandit. Aggiungi il test di regressione suggerito per uno stream terminato esternamente, incluso il percorso GOAWAY da #4771 e una race durante il completamento normale. Il lavoro è completato quando i permessi vengono recuperati esattamente una volta e una richiesta successiva può acquisire il limite.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Versions

  • Electric: current main
  • HTTP server: Bandit 1.12.0
  • Protocol: HTTP/2

Bug description

AdmissionControl permits are released by an after block in Electric.Plug.ServeShapePlug.call/2:

try do
  # Run the shape request
after
  release_admission_permit()
end

The acquired permit is stored only in the request process dictionary:

Process.put(@admission_permit_key, {stack_id, kind})

This handles normal returns, exceptions, throws, and exits raised within the request process. It does not handle the request process being terminated by an external exit signal, because the process stops without executing the after block.

This matters under Bandit's HTTP/2 process model:

  1. Each HTTP/2 stream runs its Plug pipeline in a separate Bandit.HTTP2.StreamProcess.
  2. The stream process is linked to the HTTP/2 connection process.
  3. The stream process does not trap exits.
  4. When the connection process terminates with a non-normal reason, its active stream processes terminate immediately.
  5. Any AdmissionControl permits held by those processes remain counted in the AdmissionControl ETS table.

HTTP/2 connection termination can happen when:

  • The peer closes the connection while streams are active.
  • The socket encounters an error.
  • The server closes the connection.
  • The HTTP/2 max_requests behavior described in #4771 sends GOAWAY and closes the connection.

Electric's live requests may spend up to long_poll_timeout waiting for changes, so it is normal for an HTTP/2 connection to have active request processes when the connection disappears.

Because the AdmissionControl ETS table is owned independently of the request and connection processes, leaked counts do not recover when those processes terminate. Repeated connection closures can ratchet the :initial or :existing counter upward until the configured concurrency limit is exhausted. At that point, subsequent requests receive 503 concurrent_request_limit_exceeded despite there being fewer—or no—corresponding live request processes.

The counters only recover when AdmissionControl or the VM restarts.

Expected behavior

An AdmissionControl permit should be released exactly once when its owning request process terminates, regardless of whether the request:

  • Returns normally.
  • Raises or exits internally.
  • Is terminated by an external exit signal.
  • Is killed because its HTTP/2 connection closes.

AdmissionControl's counters should represent live permit holders rather than relying exclusively on cooperative cleanup by those holders.

Suggested fix

Associate each acquired permit with its owning request process and monitor that process.

Both normal release and owner-process termination should attempt to atomically claim the permit lease before decrementing the counter. Only the path that successfully claims the lease should perform the decrement. This provides exactly-once release when normal cleanup races with a :DOWN notification.

The current direct ETS counter updates are intentionally cheap, so the implementation should avoid serializing every stack through one global GenServer. Possible implementations include partitioned lease monitors or another ownership mechanism that preserves concurrency across stacks.

Making HTTP/2 stream processes trap connection exits would be a narrower workaround, but it would only defer cleanup until the request returns and would couple Electric to Bandit's process model. Owner-monitored permits address the underlying AdmissionControl invariant regardless of why a request process terminates.

Suggested regression test

  1. Start Bandit with HTTP/2 enabled.
  2. Configure an AdmissionControl limit of one.
  3. Open a live request that remains in its long-poll wait and verify the counter is one.
  4. Close the underlying HTTP/2 connection while the stream remains active.
  5. Wait for the stream process to terminate.
  6. Assert that the AdmissionControl counter returns to zero.
  7. Assert that another request can acquire the permit.

A second test should cover the GOAWAY path from #4771 by configuring a very small HTTP/2 max_requests value.

The test should also verify that normal request completion does not cause a double release if process-death cleanup races with the existing after block.

Acceptance criteria

  • Admission permits are associated with their owning request processes.
  • A permit is reclaimed when its owner terminates unexpectedly.
  • Normal completion and owner-death cleanup cannot release the same permit twice.
  • HTTP/2 connection closure with active streams does not leave AdmissionControl counters elevated.
  • A regression test covers externally terminated HTTP/2 stream processes.
  • #4771 remains the mitigation for avoiding request-count-driven HTTP/2 connection cycling; this issue does not reintroduce an HTTP/2 request cap.
Lingua principale
TypeScript
Stelle
10.4k
Fork
399
Nessuna PR unita negli ultimi 30g

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di electric-sql/electric

Tutte le issue di electric-sql/electric

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.