Skip to content

fix: graceful HTTP server shutdown with in-flight request draining - #1210

Open
enriquevalenzuelagalaxy wants to merge 5 commits into
Consensys-Incorporated:masterfrom
GalaxyDigitalPublic:ev/upstream/graceful-http-shutdown
Open

fix: graceful HTTP server shutdown with in-flight request draining#1210
enriquevalenzuelagalaxy wants to merge 5 commits into
Consensys-Incorporated:masterfrom
GalaxyDigitalPublic:ev/upstream/graceful-http-shutdown

Conversation

@enriquevalenzuelagalaxy

@enriquevalenzuelagalaxy enriquevalenzuelagalaxy commented Jul 13, 2026

Copy link
Copy Markdown

PR Description

Adds graceful HTTP server shutdown to Runner. When close() is called, the server now waits for any in-flight HTTP requests to complete before closing the Vert.x HttpServer, preventing abrupt connection resets under load or during rolling restarts.

Changes:

  • Added trackInFlightRequests() — wraps the request handler to atomically increment/decrement an in-flight counter, guarded against double-counting via AtomicBoolean per request
  • Added waitForInFlightRequestsToComplete() — blocks the shutdown thread (up to 20 seconds) using Object.wait/notifyAll on a monitor
  • Added gracefulHttpShutdown() — drains in-flight requests then closes the HttpServer via its async close() with a latch
  • Modified close() to call gracefulHttpShutdown() before closing other resources
  • httpServer promoted from local variable to volatile instance field so close() can reference it

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

Testing

  • I thought about testing these changes in a realistic/non-local environment.

RunnerGracefulShutdownTest covers three scenarios using a real embedded Vert.x server:

  1. closeWaitsForInFlightRequestBeforeShuttingDown — verifies close() blocks until a slow in-flight request completes before returning
  2. closeCompletesNormallyWithNoInFlightRequests — verifies close() doesn't block when idle
  3. closeBeforeRunDoesNotThrow — verifies the null-guard on httpServer is correct

Note

Medium Risk
Changes core HTTP lifecycle and shutdown ordering for a signing service; bounded timeouts mean very long requests may still be cut off, but signing logic is unchanged.

Overview
Runner.close() now drains active HTTP work before tearing down other resources, so rolling restarts and shutdown under load are less likely to reset connections mid-signing.

Shutdown sets a shuttingDown flag and wraps the Vert.x request handler with trackInFlightRequests, which counts in-flight requests, rejects new ones with 503 once shutdown has started, and decrements on response completion (with guards against double-counting). gracefulHttpShutdown waits up to 20 seconds for the counter to reach zero, then closes the HttpServer asynchronously with a bounded wait. The server instance is stored on the runner so close() can run this path even when run() was never started (null-safe).

RunnerGracefulShutdownTest exercises blocking close() until a slow handler finishes, idle shutdown, and close() before run().

Reviewed by Cursor Bugbot for commit bd4bb58. Bugbot is set up for automated code reviews on this repo. Configure here.

ejether and others added 2 commits July 13, 2026 15:00
Covers three scenarios:
- close() waits for an in-flight request to complete before shutting down
- close() completes normally when no requests are in-flight
- close() before run() does not throw
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@enriquevalenzuelagalaxy

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

Comment thread core/src/main/java/tech/pegasys/web3signer/core/Runner.java
… test

- Add shuttingDown flag — trackInFlightRequests now returns 503 immediately
  when shutdown has started, preventing new requests from incrementing the
  counter after draining begins. Without this, sustained load could cause
  the drain to time out or a new request could slip in after counter=0 but
  before httpServer.close() runs.
- Replace Thread.sleep(300) in test with explicit TimeoutException assertion
  to verify close() is blocked — removes SonarQube S2925 (Bug) violation.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 3e7d4a2. Configure here.

Comment thread core/src/main/java/tech/pegasys/web3signer/core/Runner.java
@ejether

ejether commented Jul 13, 2026

Copy link
Copy Markdown

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Jul 13, 2026
@sonarqubecloud

Copy link
Copy Markdown

@usmansaleem usmansaleem self-assigned this Jul 19, 2026
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.

3 participants