Skip to content

Commit ef20528

Browse files
committed
feat: Drain in-flight requests on stop
HttpServer.stop(delay) can wait out the full delay with nothing in flight: when a client closes an idle keep-alive connection just before stop, the JDK never counts its finished latch down (JDK 25 ServerImpl). OpenApiServer now counts in-flight requests with a filter on every context it creates. stop(delay) closes the listener at once through a background HttpServer.stop(delay), waits on the counter for at most the delay, then calls HttpServer.stop(0). Later and concurrent stop calls wait for the first. inFlightRequests() exposes the count.
1 parent 3ecefdb commit ef20528

5 files changed

Lines changed: 584 additions & 9 deletions

File tree

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Request flow when `OpenApiServer` boots (`src/main/java/com/retailsvc/http/OpenA
2727

2828
1. `HttpServer` is created on a port with a virtual-thread-per-task executor.
2929
2. One `HttpContext` is registered per spec binding at `spec.basePath()` (the first `servers[].url` path from the OpenAPI doc). Unless a binding owns `/`, a catch-all `/` context serves extra routes via `ExtrasRouter` and 404s everything else; `ExceptionFilter` wraps that context only.
30-
3. On a binding context, two filters run in order, then the handler:
30+
3. Every context the server creates, the binding contexts and the extras one, starts with an `InFlightCounter` filter that counts the exchange for `OpenApiServer.inFlightRequests()` and `stop(int)`. On a binding context, two more filters run in order, then the handler:
3131
- `RequestPreparationFilter` — reads the request body through `RequestBodyReader` (which decodes a registered `Content-Encoding` — gzip is built in — under a size cap), resolves the route, runs OpenAPI parameter + body validation via `DefaultValidator`, and binds the resulting `Request` into the `DispatchHandler.CURRENT` scoped value. It renders its own failures through the `ExceptionHandler` rather than relying on `ExceptionFilter`.
3232
- `SecurityFilter` — enforces the spec's `securitySchemes` / `security`, re-binding the `Request` with resolved principals. It writes its 401/403 responses straight to the exchange.
3333
- `DispatchHandler` — looks up the `RequestHandler` registered for the resolved `operationId` in the user-supplied map and invokes it, applying interceptors and response decorators. Handler coverage is verified at boot, so the lookup never returns `null`.

‎README.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,22 @@ try (var server = OpenApiServer.builder()
574574
`stop(int)` and `shutdownTimeoutSeconds(int)` reject negative values with
575575
`IllegalArgumentException`.
576576

577+
Stopping closes the listening socket at once, so no new connection is accepted. Requests on
578+
keep-alive connections that are already open are served until the JDK sees no exchange in flight,
579+
which can come before the after-response runnables of the last request have finished; the JDK then
580+
closes every connection, keep-alive ones included. The wait ends as soon as `inFlightRequests()` reads `0`, or when the delay elapses; every connection
581+
is then closed, which cuts off what is still running. A request counts as in flight from the first
582+
filter until its after-response runnables (`Request.afterResponse`, hooks) have run, so `stop` waits
583+
for those too. A request the JDK rejects before it dispatches it, such as a malformed one, is not
584+
counted.
585+
586+
Only the first `stop` call stops the server. A later or concurrent call waits until that stop is
587+
complete, then returns (or throws `IllegalStateException` if that stop failed); its own delay is
588+
ignored. A thread interrupted while it waits stops waiting and keeps its interrupt flag.
589+
590+
`inFlightRequests()` is the server's own count, readable at any time, for example to log it or to
591+
wait for `0` before stopping.
592+
577593
## Interceptors and response decorators
578594

579595
### Response decorators

‎src/main/java/com/retailsvc/http/OpenApiServer.java‎

Lines changed: 98 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import com.retailsvc.http.internal.ExceptionFilter;
1010
import com.retailsvc.http.internal.ExtrasRouter;
1111
import com.retailsvc.http.internal.FormTypeMapper;
12+
import com.retailsvc.http.internal.InFlightCounter;
1213
import com.retailsvc.http.internal.PemSslContext;
1314
import com.retailsvc.http.internal.RequestBodyReader;
1415
import com.retailsvc.http.internal.RequestPreparationFilter;
@@ -40,6 +41,10 @@
4041
import java.util.Optional;
4142
import java.util.Set;
4243
import java.util.TreeSet;
44+
import java.util.concurrent.CompletableFuture;
45+
import java.util.concurrent.ExecutionException;
46+
import java.util.concurrent.TimeUnit;
47+
import java.util.concurrent.atomic.AtomicBoolean;
4348
import java.util.stream.Collectors;
4449
import javax.net.ssl.SSLContext;
4550
import org.slf4j.Logger;
@@ -57,9 +62,13 @@ public class OpenApiServer implements AutoCloseable {
5762
private static final int DEFAULT_HTTPS_PORT = 8443;
5863
private static final String JSON = "application/json";
5964
private static final String GSON_CLASS = "com.google.gson.Gson";
65+
private static final long STOP_JOIN_SECONDS = 5;
6066

6167
private final HttpServer httpServer;
6268
private final int shutdownTimeoutSeconds;
69+
private final InFlightCounter counter;
70+
private final AtomicBoolean stopping = new AtomicBoolean();
71+
private final CompletableFuture<Void> stopped = new CompletableFuture<>();
6372

6473
/** Internal grouping of handler-related configuration to keep the constructor signature small. */
6574
record HandlerConfig(
@@ -98,8 +107,10 @@ record HandlerConfig(
98107
this.httpServer = createHttpServer(socketAddress, sslContext);
99108
httpServer.setExecutor(newThreadPerTaskExecutor(ofVirtual().name("http-", 0).factory()));
100109

101-
boolean anyBindingAtRoot = wireBindings(httpServer, bindings, bodyMappers, handlerConfig);
102-
wireExtras(httpServer, anyBindingAtRoot, handlerConfig);
110+
this.counter = new InFlightCounter();
111+
boolean anyBindingAtRoot =
112+
wireBindings(httpServer, bindings, bodyMappers, handlerConfig, counter);
113+
wireExtras(httpServer, anyBindingAtRoot, handlerConfig, counter);
103114

104115
httpServer.start();
105116
this.shutdownTimeoutSeconds = shutdownTimeoutSeconds;
@@ -120,12 +131,13 @@ private static boolean wireBindings(
120131
HttpServer httpServer,
121132
List<SpecBinding> bindings,
122133
Map<String, TypeMapper> bodyMappers,
123-
HandlerConfig handlerConfig) {
134+
HandlerConfig handlerConfig,
135+
InFlightCounter counter) {
124136
boolean anyBindingAtRoot = false;
125137
for (SpecBinding binding : bindings) {
126138
String basePath = Optional.ofNullable(binding.spec().basePath()).orElse("/");
127139
anyBindingAtRoot |= "/".equals(basePath);
128-
wireBinding(httpServer, basePath, binding, bodyMappers, handlerConfig);
140+
wireBinding(httpServer, basePath, binding, bodyMappers, handlerConfig, counter);
129141
}
130142
return anyBindingAtRoot;
131143
}
@@ -135,11 +147,13 @@ private static void wireBinding(
135147
String basePath,
136148
SpecBinding binding,
137149
Map<String, TypeMapper> bodyMappers,
138-
HandlerConfig handlerConfig) {
150+
HandlerConfig handlerConfig,
151+
InFlightCounter counter) {
139152
Map<String, Operation> operationsById =
140153
binding.spec().operations().stream()
141154
.collect(Collectors.toUnmodifiableMap(Operation::operationId, op -> op));
142155
HttpContext ctx = httpServer.createContext(basePath);
156+
ctx.getFilters().add(counter.filter());
143157
ctx.getFilters()
144158
.add(
145159
new RequestPreparationFilter(
@@ -170,7 +184,10 @@ private static void wireBinding(
170184
}
171185

172186
private static void wireExtras(
173-
HttpServer httpServer, boolean anyBindingAtRoot, HandlerConfig handlerConfig) {
187+
HttpServer httpServer,
188+
boolean anyBindingAtRoot,
189+
HandlerConfig handlerConfig,
190+
InFlightCounter counter) {
174191
Map<String, RequestHandler> extras = handlerConfig.extras();
175192
if (anyBindingAtRoot) {
176193
if (!extras.isEmpty()) {
@@ -186,6 +203,7 @@ private static void wireExtras(
186203
handlerConfig.bodyReader(),
187204
handlerConfig.streamingLimit());
188205
HttpContext extrasCtx = httpServer.createContext("/", extrasRouter);
206+
extrasCtx.getFilters().add(counter.filter());
189207
extrasCtx
190208
.getFilters()
191209
.add(new ExceptionFilter(handlerConfig.exceptionHandler(), handlerConfig.renderer()));
@@ -213,18 +231,90 @@ public InetAddress bindAddress() {
213231
return httpServer.getAddress().getAddress();
214232
}
215233

234+
/**
235+
* The number of requests being handled now, from the first filter to the end of the
236+
* after-response hooks. A request the JDK rejects before it dispatches it, such as a malformed
237+
* one, is not counted. Reads {@code 0} once the server is idle, so a caller can wait for that
238+
* before {@link #stop(int)}.
239+
*/
240+
public int inFlightRequests() {
241+
return counter.inFlight();
242+
}
243+
216244
/**
217245
* Stops the server, waiting up to {@code delaySeconds} for active exchanges to finish before
218246
* closing them. {@code 0} stops immediately.
219247
*
248+
* <p>The listening socket closes at once, so no new connection is accepted. Requests on
249+
* keep-alive connections that are already open are served until the JDK sees no exchange in
250+
* flight, which can come before the after-response runnables of the last request have finished;
251+
* the JDK then closes every connection, keep-alive ones included. The wait ends when {@link
252+
* #inFlightRequests()} reads {@code 0} or the delay elapses, whichever comes first; the server
253+
* then closes every connection, which cuts off whatever is still running. The wait is on this
254+
* server's own count, not on the JDK's, which can wait out the whole delay with nothing in
255+
* flight.
256+
*
257+
* <p>Only the first call stops the server. A later call, also a concurrent one, waits until that
258+
* stop is complete and then returns, or throws {@link IllegalStateException} if that stop failed;
259+
* its own {@code delaySeconds} is ignored. A thread that is interrupted while it waits stops
260+
* waiting and keeps its interrupt flag.
261+
*
220262
* @param delaySeconds maximum seconds to wait for in-flight exchanges; must be non-negative
221263
*/
222264
public void stop(int delaySeconds) {
223265
if (delaySeconds < 0) {
224266
throw new IllegalArgumentException("delaySeconds must be non-negative, got " + delaySeconds);
225267
}
226-
if (httpServer != null) {
227-
httpServer.stop(delaySeconds);
268+
if (stopping.compareAndSet(false, true)) {
269+
try {
270+
drainAndStop(delaySeconds);
271+
stopped.complete(null);
272+
} catch (RuntimeException | Error e) {
273+
stopped.completeExceptionally(e);
274+
throw e;
275+
}
276+
} else {
277+
awaitStopped();
278+
}
279+
}
280+
281+
/**
282+
* {@code HttpServer.stop(delay)} closes the listening socket at once and then waits on a latch
283+
* that, in the JDK this was written against, can stay unreleased with nothing in flight. It runs
284+
* on its own thread for the first, and {@code stop(0)} ends it: {@code ServerImpl.stop} is safe
285+
* to call again while another call waits. Each call only sets {@code terminating}, posts another
286+
* {@code StopRequested} event, closes the already closed socket and waits on the latch; its
287+
* {@code finally} counts the latch down for every waiter and clears connections and timers, which
288+
* tolerates repeats.
289+
*/
290+
private void drainAndStop(int delaySeconds) {
291+
Thread background = ofVirtual().name("http-stop").start(() -> httpServer.stop(delaySeconds));
292+
counter.awaitIdle(delaySeconds, TimeUnit.SECONDS);
293+
// The JDK swallows an interrupt in stop(0), so take the flag now and set it again at the end.
294+
boolean interrupted = Thread.interrupted();
295+
httpServer.stop(0);
296+
try {
297+
background.join(Duration.ofSeconds(STOP_JOIN_SECONDS));
298+
} catch (InterruptedException _) {
299+
interrupted = true;
300+
}
301+
if (background.isAlive()) {
302+
LOG.warn(
303+
"The JDK stop thread is still running {} s after the server was stopped",
304+
STOP_JOIN_SECONDS);
305+
}
306+
if (interrupted) {
307+
Thread.currentThread().interrupt();
308+
}
309+
}
310+
311+
private void awaitStopped() {
312+
try {
313+
stopped.get();
314+
} catch (InterruptedException _) {
315+
Thread.currentThread().interrupt();
316+
} catch (ExecutionException e) {
317+
throw new IllegalStateException("The first stop failed", e.getCause());
228318
}
229319
}
230320

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package com.retailsvc.http.internal;
2+
3+
import com.sun.net.httpserver.Filter;
4+
import com.sun.net.httpserver.HttpExchange;
5+
import java.io.IOException;
6+
import java.util.concurrent.TimeUnit;
7+
import java.util.concurrent.locks.Condition;
8+
import java.util.concurrent.locks.Lock;
9+
import java.util.concurrent.locks.ReentrantLock;
10+
11+
/**
12+
* Counts the exchanges the server is handling. One counter is shared by every {@code HttpContext}
13+
* the server creates, through {@link #filter()}, installed ahead of every other filter so the
14+
* counted window covers the whole exchange: the handler, the rendering of the response and the
15+
* after-response hooks.
16+
*
17+
* <p>The server waits on this count instead of on {@code HttpServer.stop(delay)}, because the JDK
18+
* can leave that call waiting for the whole delay with nothing in flight: a client that closes an
19+
* idle keep-alive connection just as {@code stop} is called leaves the JDK's internal latch
20+
* unreleased. Thread-safe.
21+
*/
22+
public final class InFlightCounter {
23+
24+
private final Lock lock = new ReentrantLock();
25+
private final Condition idle = lock.newCondition();
26+
private final Filter filter = new CountingFilter();
27+
private int inFlight;
28+
29+
/** The filter that counts the exchanges of the context it is added to. Add it first. */
30+
public Filter filter() {
31+
return filter;
32+
}
33+
34+
/** The number of exchanges being handled now. */
35+
public int inFlight() {
36+
lock.lock();
37+
try {
38+
return inFlight;
39+
} finally {
40+
lock.unlock();
41+
}
42+
}
43+
44+
/**
45+
* Waits until no exchange is in flight, for at most {@code timeout} of {@code unit}. If the
46+
* thread is interrupted the wait ends and the interrupt flag is set again.
47+
*
48+
* @return whether the count is zero when the wait ends
49+
*/
50+
public boolean awaitIdle(long timeout, TimeUnit unit) {
51+
long remaining = unit.toNanos(timeout);
52+
lock.lock();
53+
try {
54+
while (inFlight > 0 && remaining > 0) {
55+
remaining = idle.awaitNanos(remaining);
56+
}
57+
} catch (InterruptedException _) {
58+
Thread.currentThread().interrupt();
59+
} finally {
60+
lock.unlock();
61+
}
62+
return inFlight() == 0;
63+
}
64+
65+
private void enter() {
66+
lock.lock();
67+
try {
68+
inFlight++;
69+
} finally {
70+
lock.unlock();
71+
}
72+
}
73+
74+
private void exit() {
75+
lock.lock();
76+
try {
77+
if (--inFlight == 0) {
78+
idle.signalAll();
79+
}
80+
} finally {
81+
lock.unlock();
82+
}
83+
}
84+
85+
private final class CountingFilter extends Filter {
86+
87+
@Override
88+
public void doFilter(HttpExchange exchange, Chain chain) throws IOException {
89+
enter();
90+
try {
91+
chain.doFilter(exchange);
92+
} finally {
93+
exit();
94+
}
95+
}
96+
97+
@Override
98+
public String description() {
99+
return "In-flight counter";
100+
}
101+
}
102+
}

0 commit comments

Comments
 (0)