Fine-tuned concurrency and general polishing in SSE support classes

Issue: SPR-12212

Author: jhoeller

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

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.servlet.mvc.method.annotation;
 
 import java.io.IOException;
@@ -23,7 +24,6 @@
 import org.springframework.http.server.ServerHttpResponse;
 import org.springframework.util.Assert;
 
-
 /**
  * A controller method return value type for asynchronous request processing
  * where one or more objects are written to the response. While
@@ -53,15 +53,12 @@
  * emitter.complete();
  * </pre>
  *
- * <p><strong>Note:</strong> this class is not thread-safe. Callers must ensure
- * that use from multiple threads is synchronized.
- *
  * @author Rossen Stoyanchev
  * @since 4.2
  */
 public class ResponseBodyEmitter {
 
-	private Handler handler;
+	private volatile Handler handler;
 
 	/* Cache for objects sent before handler is set. */
 	private final Map<Object, MediaType> initHandlerCache = new LinkedHashMap<Object, MediaType>(10);
@@ -124,17 +121,17 @@ public void send(Object object) throws IOException {
 	 * @throws java.lang.IllegalStateException wraps any other errors
 	 */
 	public void send(Object object, MediaType mediaType) throws IOException {
-		Assert.state(!this.complete, "ResponseBodyEmitter is already set complete.");
+		Assert.state(!this.complete, "ResponseBodyEmitter is already set complete");
 		sendInternal(object, mediaType);
 	}
 
 	private void sendInternal(Object object, MediaType mediaType) throws IOException {
 		if (object == null) {
 			return;
 		}
-		if (handler == null) {
+		if (this.handler == null) {
 			synchronized (this) {
-				if (handler == null) {
+				if (this.handler == null) {
 					this.initHandlerCache.put(object, mediaType);
 					return;
 				}
@@ -143,11 +140,11 @@ private void sendInternal(Object object, MediaType mediaType) throws IOException
 		try {
 			this.handler.send(object, mediaType);
 		}
-		catch(IOException ex){
+		catch (IOException ex){
 			this.handler.completeWithError(ex);
 			throw ex;
 		}
-		catch(Throwable ex){
+		catch (Throwable ex){
 			this.handler.completeWithError(ex);
 			throw new IllegalStateException("Failed to send " + object, ex);
 		}
@@ -161,7 +158,7 @@ private void sendInternal(Object object, MediaType mediaType) throws IOException
 	public void complete() {
 		synchronized (this) {
 			this.complete = true;
-			if (handler != null) {
+			if (this.handler != null) {
 				this.handler.complete();
 			}
 		}
@@ -176,7 +173,7 @@ public void completeWithError(Throwable ex) {
 		synchronized (this) {
 			this.complete = true;
 			this.failure = ex;
-			if (handler != null) {
+			if (this.handler != null) {
 				this.handler.completeWithError(ex);
 			}
 		}

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

@@ -13,18 +13,17 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.servlet.mvc.method.annotation;
 
 import java.io.IOException;
 import java.io.OutputStream;
-import java.lang.reflect.ParameterizedType;
-import java.lang.reflect.Type;
 import java.util.List;
-
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ResolvableType;
 import org.springframework.http.HttpHeaders;
@@ -41,7 +40,6 @@
 import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
 import org.springframework.web.method.support.ModelAndViewContainer;
 
-
 /**
  * Supports return values of type {@link ResponseBodyEmitter} and also
  * {@code ResponseEntity<ResponseBodyEmitter>}.
@@ -61,6 +59,7 @@ public ResponseBodyEmitterReturnValueHandler(List<HttpMessageConverter<?>> messa
 		this.messageConverters = messageConverters;
 	}
 
+
 	@Override
 	public boolean supportsReturnType(MethodParameter returnType) {
 		if (ResponseBodyEmitter.class.isAssignableFrom(returnType.getParameterType())) {
@@ -121,13 +120,11 @@ private class HttpMessageConvertingHandler implements ResponseBodyEmitter.Handle
 
 		private final DeferredResult<?> deferredResult;
 
-
 		public HttpMessageConvertingHandler(ServerHttpResponse outputMessage, DeferredResult<?> deferredResult) {
 			this.outputMessage = outputMessage;
 			this.deferredResult = deferredResult;
 		}
 
-
 		@Override
 		public void send(Object data, MediaType mediaType) throws IOException {
 			sendInternal(data, mediaType);
@@ -145,7 +142,7 @@ private <T> void sendInternal(T data, MediaType mediaType) throws IOException {
 					return;
 				}
 			}
-			throw new IllegalArgumentException("No suitable converter for " + data);
+			throw new IllegalArgumentException("No suitable converter for " + data.getClass());
 		}
 
 		@Override
@@ -170,13 +167,11 @@ private static class StreamingServletServerHttpResponse implements ServerHttpRes
 
 		private final HttpHeaders mutableHeaders = new HttpHeaders();
 
-
 		public StreamingServletServerHttpResponse(ServerHttpResponse delegate) {
 			this.delegate = delegate;
 			this.mutableHeaders.putAll(delegate.getHeaders());
 		}
 
-
 		@Override
 		public void setStatusCode(HttpStatus status) {
 			this.delegate.setStatusCode(status);

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

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.servlet.mvc.method.annotation;
 
 import java.io.IOException;
@@ -26,17 +27,15 @@
 import org.springframework.http.server.ServerHttpResponse;
 
 /**
- * A specialization of
- * {@link org.springframework.web.servlet.mvc.method.annotation.ResponseBodyEmitter
- * ResponseBodyEmitter} for sending
+ * A specialization of {@link ResponseBodyEmitter} for sending
  * <a href="http://www.w3.org/TR/eventsource/">Server-Sent Events</a>.
  *
  * @author Rossen Stoyanchev
  * @since 4.2
  */
 public class SseEmitter extends ResponseBodyEmitter {
 
-	public static final MediaType TEXT_PLAIN = new MediaType("text", "plain", Charset.forName("UTF-8"));
+	static final MediaType TEXT_PLAIN = new MediaType("text", "plain", Charset.forName("UTF-8"));
 
 
 	@Override
@@ -51,7 +50,6 @@ protected void extendResponse(ServerHttpResponse outputMessage) {
 	/**
 	 * Send the object formatted as a single SSE "data" line. It's equivalent to:
 	 * <pre>
-	 *
 	 * // static import of SseEmitter.*
 	 *
 	 * SseEmitter emitter = new SseEmitter();
@@ -69,7 +67,6 @@ public void send(Object object) throws IOException {
 	/**
 	 * Send the object formatted as a single SSE "data" line. It's equivalent to:
 	 * <pre>
-	 *
 	 * // static import of SseEmitter.*
 	 *
 	 * SseEmitter emitter = new SseEmitter();
@@ -91,7 +88,6 @@ public void send(Object object, MediaType mediaType) throws IOException {
 	/**
 	 * Send an SSE event prepared with the given builder. For example:
 	 * <pre>
-	 *
 	 * // static import of SseEmitter
 	 *
 	 * SseEmitter emitter = new SseEmitter();
@@ -108,6 +104,7 @@ public void send(SseEventBuilder builder) throws IOException {
 		}
 	}
 
+
 	public static SseEventBuilder event() {
 		return new DefaultSseEventBuilder();
 	}
@@ -156,6 +153,7 @@ public interface SseEventBuilder {
 		Map<Object, MediaType> build();
 	}
 
+
 	/**
 	 * Default implementation of SseEventBuilder.
 	 */
@@ -165,7 +163,6 @@ private static class DefaultSseEventBuilder implements SseEventBuilder {
 
 		private StringBuilder sb;
 
-
 		@Override
 		public SseEventBuilder comment(String comment) {
 			append(":").append(comment != null ? comment : "").append("\n");