From 4becce1c2b1b372b44bf29b69ab3c08798214445 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 18 Jul 2023 10:22:45 +0100 Subject: [PATCH] Consolidate default WebMvc executor log warnings Closes gh-30902 --- .../request/async/WebAsyncManager.java | 26 -------------- .../annotation/ReactiveTypeHandler.java | 30 ---------------- .../RequestMappingHandlerAdapter.java | 35 ++++++++++++++++++- 3 files changed, 34 insertions(+), 57 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java index 6f7be997a2..de35961e20 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java @@ -30,7 +30,6 @@ import org.apache.commons.logging.LogFactory; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.core.task.SimpleAsyncTaskExecutor; -import org.springframework.core.task.SyncTaskExecutor; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.web.context.request.RequestAttributes; @@ -74,8 +73,6 @@ public final class WebAsyncManager { private static final DeferredResultProcessingInterceptor timeoutDeferredResultInterceptor = new TimeoutDeferredResultProcessingInterceptor(); - private static Boolean taskExecutorWarning = true; - @Nullable private AsyncWebRequest asyncWebRequest; @@ -295,9 +292,6 @@ public final class WebAsyncManager { if (executor != null) { this.taskExecutor = executor; } - else { - logExecutorWarning(); - } List interceptors = new ArrayList<>(); interceptors.add(webAsyncTask.getInterceptor()); @@ -357,26 +351,6 @@ public final class WebAsyncManager { } } - private void logExecutorWarning() { - if (taskExecutorWarning && logger.isWarnEnabled()) { - synchronized (DEFAULT_TASK_EXECUTOR) { - AsyncTaskExecutor executor = this.taskExecutor; - if (taskExecutorWarning && - (executor instanceof SimpleAsyncTaskExecutor || executor instanceof SyncTaskExecutor)) { - String executorTypeName = executor.getClass().getSimpleName(); - logger.warn("\n!!!\n" + - "An Executor is required to handle java.util.concurrent.Callable return values.\n" + - "Please, configure a TaskExecutor in the MVC config under \"async support\".\n" + - "The " + executorTypeName + " currently in use is not suitable under load.\n" + - "-------------------------------\n" + - "Request URI: '" + formatRequestUri() + "'\n" + - "!!!"); - taskExecutorWarning = false; - } - } - } - } - private String formatRequestUri() { HttpServletRequest request = this.asyncWebRequest.getNativeRequest(HttpServletRequest.class); return request != null ? request.getRequestURI() : "servlet container"; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java index 8528b955ec..9bf2345f37 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ReactiveTypeHandler.java @@ -38,7 +38,6 @@ import org.springframework.core.MethodParameter; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.ResolvableType; -import org.springframework.core.task.SimpleAsyncTaskExecutor; import org.springframework.core.task.SyncTaskExecutor; import org.springframework.core.task.TaskExecutor; import org.springframework.http.MediaType; @@ -91,8 +90,6 @@ class ReactiveTypeHandler { private final ContentNegotiationManager contentNegotiationManager; - private boolean taskExecutorWarning; - public ReactiveTypeHandler() { this(ReactiveAdapterRegistry.getSharedInstance(), new SyncTaskExecutor(), new ContentNegotiationManager()); @@ -105,9 +102,6 @@ class ReactiveTypeHandler { this.adapterRegistry = registry; this.taskExecutor = executor; this.contentNegotiationManager = manager; - - this.taskExecutorWarning = - (executor instanceof SimpleAsyncTaskExecutor || executor instanceof SyncTaskExecutor); } @@ -147,20 +141,17 @@ class ReactiveTypeHandler { if (adapter.isMultiValue()) { if (mediaTypes.stream().anyMatch(MediaType.TEXT_EVENT_STREAM::includes) || ServerSentEvent.class.isAssignableFrom(elementClass)) { - logExecutorWarning(returnType); SseEmitter emitter = new SseEmitter(STREAMING_TIMEOUT_VALUE); new SseEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue); return emitter; } if (CharSequence.class.isAssignableFrom(elementClass)) { - logExecutorWarning(returnType); ResponseBodyEmitter emitter = getEmitter(mediaType.orElse(MediaType.TEXT_PLAIN)); new TextEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue); return emitter; } MediaType streamingResponseType = findConcreteStreamingMediaType(mediaTypes); if (streamingResponseType != null) { - logExecutorWarning(returnType); ResponseBodyEmitter emitter = getEmitter(streamingResponseType); new JsonEmitterSubscriber(emitter, this.taskExecutor).connect(adapter, returnValue); return emitter; @@ -234,27 +225,6 @@ class ReactiveTypeHandler { }; } - @SuppressWarnings("ConstantConditions") - private void logExecutorWarning(MethodParameter returnType) { - if (this.taskExecutorWarning && logger.isWarnEnabled()) { - synchronized (this) { - if (this.taskExecutorWarning) { - String executorTypeName = this.taskExecutor.getClass().getSimpleName(); - logger.warn("\n!!!\n" + - "Streaming through a reactive type requires an Executor to write to the response.\n" + - "Please, configure a TaskExecutor in the MVC config under \"async support\".\n" + - "The " + executorTypeName + " currently in use is not suitable under load.\n" + - "-------------------------------\n" + - "Controller:\t" + returnType.getContainingClass().getName() + "\n" + - "Method:\t\t" + returnType.getMethod().getName() + "\n" + - "Returning:\t" + ResolvableType.forMethodParameter(returnType) + "\n" + - "!!!"); - this.taskExecutorWarning = false; - } - } - } - } - private abstract static class AbstractEmitterSubscriber implements Subscriber, Runnable { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 17420c5bff..733dea43c4 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -169,7 +169,7 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter @Nullable private MethodValidator methodValidator; - private AsyncTaskExecutor taskExecutor = new SimpleAsyncTaskExecutor("MvcAsync"); + private AsyncTaskExecutor taskExecutor = new MvcSimpleAsyncTaskExecutor(); @Nullable private Long asyncRequestTimeout; @@ -1041,4 +1041,37 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter return mav; } + + /** + * A default Spring MVC AsyncTaskExecutor that warns if used. + */ + @SuppressWarnings("serial") + private class MvcSimpleAsyncTaskExecutor extends SimpleAsyncTaskExecutor { + + private static Boolean taskExecutorWarning = true; + + public MvcSimpleAsyncTaskExecutor() { + super("MvcAsync"); + } + + @Override + public void execute(Runnable task) { + if (taskExecutorWarning && logger.isWarnEnabled()) { + synchronized (this) { + if (taskExecutorWarning) { + logger.warn(""" + !!! + Performing asynchronous handling through the default Spring MVC SimpleAsyncTaskExecutor. + This executor is not suitable for production use under load. + Please, configure an AsyncTaskExecutor through the WebMvc config. + ------------------------------- + !!!"""); + taskExecutorWarning = false; + } + } + } + super.execute(task); + } + } + }