Skip to content

Commit c2137a0

Browse files
committed
Cancel handling onError/Timeout in ServletHttpHandlerAdapter
This commit ensures handling is cancelled in case of onError/Timeout callback from the Servlet container. Separately we detect the same in ServletServerHttpRequest and ServletServerHttpResponse, which signal onError to the read publisher and cancel writing, but if the onError/Timeout arrives after reading is done and before writing has started (e.g. longer handling), then neither will reach handling. See gh-26434, gh-26407
1 parent 8c9c59e commit c2137a0

File tree

1 file changed

+37
-12
lines changed

1 file changed

+37
-12
lines changed

spring-web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java

+37-12
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,10 @@ public void service(ServletRequest request, ServletResponse response) throws Ser
185185
}
186186

187187
AtomicBoolean isCompleted = new AtomicBoolean();
188-
HandlerResultAsyncListener listener = new HandlerResultAsyncListener(isCompleted, httpRequest);
189-
asyncContext.addListener(listener);
190-
191188
HandlerResultSubscriber subscriber = new HandlerResultSubscriber(asyncContext, isCompleted, httpRequest);
189+
HandlerResultAsyncListener listener = new HandlerResultAsyncListener(isCompleted, httpRequest, subscriber);
190+
191+
asyncContext.addListener(listener);
192192
this.httpHandler.handle(httpRequest, httpResponse).subscribe(subscriber);
193193
}
194194

@@ -222,10 +222,6 @@ public void destroy() {
222222
}
223223

224224

225-
/**
226-
* We cannot combine ERROR_LISTENER and HandlerResultSubscriber due to:
227-
* https://issues.jboss.org/browse/WFLY-8515.
228-
*/
229225
private static void runIfAsyncNotComplete(AsyncContext asyncContext, AtomicBoolean isCompleted, Runnable task) {
230226
try {
231227
if (asyncContext.getRequest().isAsyncStarted() && isCompleted.compareAndSet(false, true)) {
@@ -254,24 +250,41 @@ private static class HandlerResultAsyncListener implements AsyncListener {
254250

255251
private final String logPrefix;
256252

257-
public HandlerResultAsyncListener(AtomicBoolean isCompleted, ServletServerHttpRequest request) {
253+
// We cannot have AsyncListener and HandlerResultSubscriber until WildFly 12+:
254+
// https://issues.jboss.org/browse/WFLY-8515
255+
private final Runnable handlerDisposeTask;
256+
257+
public HandlerResultAsyncListener(
258+
AtomicBoolean isCompleted, ServletServerHttpRequest request, Runnable handlerDisposeTask) {
259+
258260
this.isCompleted = isCompleted;
259261
this.logPrefix = request.getLogPrefix();
262+
this.handlerDisposeTask = handlerDisposeTask;
260263
}
261264

262265
@Override
263266
public void onTimeout(AsyncEvent event) {
264267
logger.debug(this.logPrefix + "Timeout notification");
265-
AsyncContext context = event.getAsyncContext();
266-
runIfAsyncNotComplete(context, this.isCompleted, context::complete);
268+
handleTimeoutOrError(event);
267269
}
268270

269271
@Override
270272
public void onError(AsyncEvent event) {
271273
Throwable ex = event.getThrowable();
272274
logger.debug(this.logPrefix + "Error notification: " + (ex != null ? ex : "<no Throwable>"));
275+
handleTimeoutOrError(event);
276+
}
277+
278+
private void handleTimeoutOrError(AsyncEvent event) {
273279
AsyncContext context = event.getAsyncContext();
274-
runIfAsyncNotComplete(context, this.isCompleted, context::complete);
280+
runIfAsyncNotComplete(context, this.isCompleted, () -> {
281+
try {
282+
this.handlerDisposeTask.run();
283+
}
284+
finally {
285+
context.complete();
286+
}
287+
});
275288
}
276289

277290
@Override
@@ -286,14 +299,17 @@ public void onComplete(AsyncEvent event) {
286299
}
287300

288301

289-
private static class HandlerResultSubscriber implements Subscriber<Void> {
302+
private static class HandlerResultSubscriber implements Subscriber<Void>, Runnable {
290303

291304
private final AsyncContext asyncContext;
292305

293306
private final AtomicBoolean isCompleted;
294307

295308
private final String logPrefix;
296309

310+
@Nullable
311+
private volatile Subscription subscription;
312+
297313
public HandlerResultSubscriber(
298314
AsyncContext asyncContext, AtomicBoolean isCompleted, ServletServerHttpRequest httpRequest) {
299315

@@ -304,6 +320,7 @@ public HandlerResultSubscriber(
304320

305321
@Override
306322
public void onSubscribe(Subscription subscription) {
323+
this.subscription = subscription;
307324
subscription.request(Long.MAX_VALUE);
308325
}
309326

@@ -339,6 +356,14 @@ public void onComplete() {
339356
logger.trace(this.logPrefix + "Handling completed");
340357
runIfAsyncNotComplete(this.asyncContext, this.isCompleted, this.asyncContext::complete);
341358
}
359+
360+
@Override
361+
public void run() {
362+
Subscription s = this.subscription;
363+
if (s != null) {
364+
s.cancel();
365+
}
366+
}
342367
}
343368

344369
}

0 commit comments

Comments
 (0)