Skip to content

Commit 5a44897

Browse files
rstoyanchevrstoyanchev
committed
Polishing in WebAsyncManager
See gh-34192
1 parent 181db1d commit 5a44897

File tree

4 files changed

+105
-95
lines changed

4 files changed

+105
-95
lines changed

‎spring-web/src/main/java/org/springframework/web/context/request/async/DeferredResult.java

+58-48
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -288,65 +288,75 @@ public boolean setErrorResult(Object result) {
288288
}
289289

290290

291-
final DeferredResultProcessingInterceptor getInterceptor() {
292-
return new DeferredResultProcessingInterceptor() {
293-
@Override
294-
public <S> boolean handleTimeout(NativeWebRequest request, DeferredResult<S> deferredResult) {
295-
boolean continueProcessing = true;
296-
try {
297-
if (timeoutCallback != null) {
298-
timeoutCallback.run();
299-
}
300-
}
301-
finally {
302-
Object value = timeoutResult.get();
303-
if (value != RESULT_NONE) {
304-
continueProcessing = false;
305-
try {
306-
setResultInternal(value);
307-
}
308-
catch (Throwable ex) {
309-
logger.debug("Failed to handle timeout result", ex);
310-
}
311-
}
291+
final DeferredResultProcessingInterceptor getLifecycleInterceptor() {
292+
return new LifecycleInterceptor();
293+
}
294+
295+
296+
/**
297+
* Handles a DeferredResult value when set.
298+
*/
299+
@FunctionalInterface
300+
public interface DeferredResultHandler {
301+
302+
void handleResult(@Nullable Object result);
303+
}
304+
305+
306+
/**
307+
* Instance interceptor to receive Servlet container notifications.
308+
*/
309+
private class LifecycleInterceptor implements DeferredResultProcessingInterceptor {
310+
311+
@Override
312+
public <S> boolean handleTimeout(NativeWebRequest request, DeferredResult<S> result) {
313+
boolean continueProcessing = true;
314+
try {
315+
if (timeoutCallback != null) {
316+
timeoutCallback.run();
312317
}
313-
return continueProcessing;
314318
}
315-
@Override
316-
public <S> boolean handleError(NativeWebRequest request, DeferredResult<S> deferredResult, Throwable t) {
317-
try {
318-
if (errorCallback != null) {
319-
errorCallback.accept(t);
320-
}
321-
}
322-
finally {
319+
finally {
320+
Object value = timeoutResult.get();
321+
if (value != RESULT_NONE) {
322+
continueProcessing = false;
323323
try {
324-
setResultInternal(t);
324+
setResultInternal(value);
325325
}
326326
catch (Throwable ex) {
327-
logger.debug("Failed to handle error result", ex);
327+
logger.debug("Failed to handle timeout result", ex);
328328
}
329329
}
330-
return false;
331330
}
332-
@Override
333-
public <S> void afterCompletion(NativeWebRequest request, DeferredResult<S> deferredResult) {
334-
expired = true;
335-
if (completionCallback != null) {
336-
completionCallback.run();
331+
return continueProcessing;
332+
}
333+
334+
@Override
335+
public <S> boolean handleError(NativeWebRequest request, DeferredResult<S> result, Throwable t) {
336+
try {
337+
if (errorCallback != null) {
338+
errorCallback.accept(t);
337339
}
338340
}
339-
};
340-
}
341-
341+
finally {
342+
try {
343+
setResultInternal(t);
344+
}
345+
catch (Throwable ex) {
346+
logger.debug("Failed to handle error result", ex);
347+
}
348+
}
349+
return false;
350+
}
342351

343-
/**
344-
* Handles a DeferredResult value when set.
345-
*/
346-
@FunctionalInterface
347-
public interface DeferredResultHandler {
352+
@Override
353+
public <S> void afterCompletion(NativeWebRequest request, DeferredResult<S> result) {
354+
expired = true;
355+
if (completionCallback != null) {
356+
completionCallback.run();
357+
}
358+
}
348359

349-
void handleResult(@Nullable Object result);
350360
}
351361

352362
}

‎spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java

+38-38
Original file line numberDiff line numberDiff line change
@@ -383,36 +383,6 @@ public void startCallableProcessing(final WebAsyncTask<?> webAsyncTask, Object..
383383
}
384384
}
385385

386-
private void setConcurrentResultAndDispatch(@Nullable Object result) {
387-
Assert.state(this.asyncWebRequest != null, "AsyncWebRequest must not be null");
388-
synchronized (WebAsyncManager.this) {
389-
if (!this.state.compareAndSet(State.ASYNC_PROCESSING, State.RESULT_SET)) {
390-
if (logger.isDebugEnabled()) {
391-
logger.debug("Async result already set: [" + this.state.get() +
392-
"], ignored result for " + formatUri(this.asyncWebRequest));
393-
}
394-
return;
395-
}
396-
397-
this.concurrentResult = result;
398-
if (logger.isDebugEnabled()) {
399-
logger.debug("Async result set for " + formatUri(this.asyncWebRequest));
400-
}
401-
402-
if (this.asyncWebRequest.isAsyncComplete()) {
403-
if (logger.isDebugEnabled()) {
404-
logger.debug("Async request already completed for " + formatUri(this.asyncWebRequest));
405-
}
406-
return;
407-
}
408-
409-
if (logger.isDebugEnabled()) {
410-
logger.debug("Performing async dispatch for " + formatUri(this.asyncWebRequest));
411-
}
412-
this.asyncWebRequest.dispatch();
413-
}
414-
}
415-
416386
/**
417387
* Start concurrent request processing and initialize the given
418388
* {@link DeferredResult} with a {@link DeferredResultHandler} that saves
@@ -445,7 +415,7 @@ public void startDeferredResultProcessing(
445415
}
446416

447417
List<DeferredResultProcessingInterceptor> interceptors = new ArrayList<>();
448-
interceptors.add(deferredResult.getInterceptor());
418+
interceptors.add(deferredResult.getLifecycleInterceptor());
449419
interceptors.addAll(this.deferredResultInterceptors.values());
450420
interceptors.add(timeoutDeferredResultInterceptor);
451421

@@ -510,6 +480,36 @@ private void startAsyncProcessing(Object[] processingContext) {
510480
this.asyncWebRequest.startAsync();
511481
}
512482

483+
private void setConcurrentResultAndDispatch(@Nullable Object result) {
484+
Assert.state(this.asyncWebRequest != null, "AsyncWebRequest must not be null");
485+
synchronized (WebAsyncManager.this) {
486+
if (!this.state.compareAndSet(State.ASYNC_PROCESSING, State.RESULT_SET)) {
487+
if (logger.isDebugEnabled()) {
488+
logger.debug("Async result already set: [" + this.state.get() + "], " +
489+
"ignored result for " + formatUri(this.asyncWebRequest));
490+
}
491+
return;
492+
}
493+
494+
this.concurrentResult = result;
495+
if (logger.isDebugEnabled()) {
496+
logger.debug("Async result set for " + formatUri(this.asyncWebRequest));
497+
}
498+
499+
if (this.asyncWebRequest.isAsyncComplete()) {
500+
if (logger.isDebugEnabled()) {
501+
logger.debug("Async request already completed for " + formatUri(this.asyncWebRequest));
502+
}
503+
return;
504+
}
505+
506+
if (logger.isDebugEnabled()) {
507+
logger.debug("Performing async dispatch for " + formatUri(this.asyncWebRequest));
508+
}
509+
this.asyncWebRequest.dispatch();
510+
}
511+
}
512+
513513
private static String formatUri(AsyncWebRequest asyncWebRequest) {
514514
HttpServletRequest request = asyncWebRequest.getNativeRequest(HttpServletRequest.class);
515515
return (request != null ? "\"" + request.getRequestURI() + "\"" : "servlet container");
@@ -519,13 +519,13 @@ private static String formatUri(AsyncWebRequest asyncWebRequest) {
519519
/**
520520
* Represents a state for {@link WebAsyncManager} to be in.
521521
* <p><pre>
522-
* NOT_STARTED <------+
523-
* | |
524-
* v |
525-
* ASYNC_PROCESSING |
526-
* | |
527-
* v |
528-
* RESULT_SET -------+
522+
* +------> NOT_STARTED <------+
523+
* | | |
524+
* | v |
525+
* | ASYNC_PROCESSING |
526+
* | | |
527+
* | v |
528+
* <-------+ RESULT_SET -------+
529529
* </pre>
530530
* @since 5.3.33
531531
*/

‎spring-web/src/test/java/org/springframework/web/context/request/async/DeferredResultTests.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -93,7 +93,7 @@ void onCompletion() throws Exception {
9393
DeferredResult<String> result = new DeferredResult<>();
9494
result.onCompletion(() -> sb.append("completion event"));
9595

96-
result.getInterceptor().afterCompletion(null, null);
96+
result.getLifecycleInterceptor().afterCompletion(null, null);
9797

9898
assertThat(result.isSetOrExpired()).isTrue();
9999
assertThat(sb.toString()).isEqualTo("completion event");
@@ -109,7 +109,7 @@ void onTimeout() throws Exception {
109109
result.setResultHandler(handler);
110110
result.onTimeout(() -> sb.append("timeout event"));
111111

112-
result.getInterceptor().handleTimeout(null, null);
112+
result.getLifecycleInterceptor().handleTimeout(null, null);
113113

114114
assertThat(sb.toString()).isEqualTo("timeout event");
115115
assertThat(result.setResult("hello")).as("Should not be able to set result a second time").isFalse();
@@ -127,7 +127,7 @@ void onError() throws Exception {
127127
Exception e = new Exception();
128128
result.onError(t -> sb.append("error event"));
129129

130-
result.getInterceptor().handleError(null, null, e);
130+
result.getLifecycleInterceptor().handleError(null, null, e);
131131

132132
assertThat(sb.toString()).isEqualTo("error event");
133133
assertThat(result.setResult("hello")).as("Should not be able to set result a second time").isFalse();

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -360,8 +360,8 @@ public void run() {
360360
this.subscription.request(1);
361361
}
362362
catch (final Throwable ex) {
363-
if (logger.isTraceEnabled()) {
364-
logger.trace("Send for " + this.emitter + " failed: " + ex);
363+
if (logger.isDebugEnabled()) {
364+
logger.debug("Send for " + this.emitter + " failed: " + ex);
365365
}
366366
terminate();
367367
this.emitter.completeWithError(ex);
@@ -374,8 +374,8 @@ public void run() {
374374
Throwable ex = this.error;
375375
this.error = null;
376376
if (ex != null) {
377-
if (logger.isTraceEnabled()) {
378-
logger.trace("Publisher for " + this.emitter + " failed: " + ex);
377+
if (logger.isDebugEnabled()) {
378+
logger.debug("Publisher for " + this.emitter + " failed: " + ex);
379379
}
380380
this.emitter.completeWithError(ex);
381381
}

0 commit comments

Comments
 (0)