diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java index b8f767c577..96dc905c53 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java @@ -21,6 +21,7 @@ import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; @@ -211,6 +212,38 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements } + /** + * Return 0 when there is no need to obtain a lock (no async handling in + * progress), 1 if lock was acquired, and -1 if lock is not acquired because + * request is no longer usable. + */ + private int tryObtainLock() { + + if (this.state == State.NEW) { + return 0; + } + + // Do not wait indefinitely, stop if we moved on from ASYNC state (e.g. to ERROR), + // helps to avoid ABBA deadlock with onError callback + + while (this.state == State.ASYNC) { + try { + if (this.stateLock.tryLock(500, TimeUnit.MILLISECONDS)) { + if (this.state == State.ASYNC) { + return 1; + } + this.stateLock.unlock(); + break; + } + } + catch (InterruptedException ex) { + // ignore + } + } + + return -1; + } + /** * Package private access for testing only. */ @@ -244,7 +277,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public ServletOutputStream getOutputStream() throws IOException { - int level = obtainLockAndCheckState(); + int level = obtainLockOrRaiseException(); try { if (this.outputStream == null) { Assert.notNull(this.asyncWebRequest, "Not initialized"); @@ -263,7 +296,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public PrintWriter getWriter() throws IOException { - int level = obtainLockAndCheckState(); + int level = obtainLockOrRaiseException(); try { if (this.writer == null) { Assert.notNull(this.asyncWebRequest, "Not initialized"); @@ -281,7 +314,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public void flushBuffer() throws IOException { - int level = obtainLockAndCheckState(); + int level = obtainLockOrRaiseException(); try { getResponse().flushBuffer(); } @@ -293,25 +326,15 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements } } - /** - * Return 0 if checks passed and lock is not needed, 1 if checks passed - * and lock is held, or raise AsyncRequestNotUsableException. - */ - private int obtainLockAndCheckState() throws AsyncRequestNotUsableException { + private int obtainLockOrRaiseException() throws AsyncRequestNotUsableException { Assert.notNull(this.asyncWebRequest, "Not initialized"); - if (this.asyncWebRequest.state == State.NEW) { - return 0; + int result = this.asyncWebRequest.tryObtainLock(); + if (result == -1) { + throw new AsyncRequestNotUsableException("Response not usable after " + + (this.asyncWebRequest.state == State.COMPLETED ? + "async request completion" : "response errors") + "."); } - - this.asyncWebRequest.stateLock.lock(); - if (this.asyncWebRequest.state == State.ASYNC) { - return 1; - } - - this.asyncWebRequest.stateLock.unlock(); - throw new AsyncRequestNotUsableException("Response not usable after " + - (this.asyncWebRequest.state == State.COMPLETED ? - "async request completion" : "response errors") + "."); + return result; } void handleIOException(IOException ex, String msg) throws AsyncRequestNotUsableException { @@ -357,7 +380,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public void write(int b) throws IOException { - int level = this.response.obtainLockAndCheckState(); + int level = this.response.obtainLockOrRaiseException(); try { this.delegate.write(b); } @@ -370,7 +393,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements } public void write(byte[] buf, int offset, int len) throws IOException { - int level = this.response.obtainLockAndCheckState(); + int level = this.response.obtainLockOrRaiseException(); try { this.delegate.write(buf, offset, len); } @@ -384,7 +407,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public void flush() throws IOException { - int level = this.response.obtainLockAndCheckState(); + int level = this.response.obtainLockOrRaiseException(); try { this.delegate.flush(); } @@ -398,7 +421,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public void close() throws IOException { - int level = this.response.obtainLockAndCheckState(); + int level = this.response.obtainLockOrRaiseException(); try { this.delegate.close(); } @@ -432,7 +455,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public void flush() { - int level = tryObtainLockAndCheckState(); + int level = this.asyncWebRequest.tryObtainLock(); if (level > -1) { try { this.delegate.flush(); @@ -445,7 +468,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public void close() { - int level = tryObtainLockAndCheckState(); + int level = this.asyncWebRequest.tryObtainLock(); if (level > -1) { try { this.delegate.close(); @@ -463,7 +486,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public void write(int c) { - int level = tryObtainLockAndCheckState(); + int level = this.asyncWebRequest.tryObtainLock(); if (level > -1) { try { this.delegate.write(c); @@ -476,7 +499,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public void write(char[] buf, int off, int len) { - int level = tryObtainLockAndCheckState(); + int level = this.asyncWebRequest.tryObtainLock(); if (level > -1) { try { this.delegate.write(buf, off, len); @@ -494,7 +517,7 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @Override public void write(String s, int off, int len) { - int level = tryObtainLockAndCheckState(); + int level = this.asyncWebRequest.tryObtainLock(); if (level > -1) { try { this.delegate.write(s, off, len); @@ -510,22 +533,6 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements this.delegate.write(s); } - /** - * Return 0 if checks passed and lock is not needed, 1 if checks passed - * and lock is held, and -1 if checks did not pass. - */ - private int tryObtainLockAndCheckState() { - if (this.asyncWebRequest.state == State.NEW) { - return 0; - } - this.asyncWebRequest.stateLock.lock(); - if (this.asyncWebRequest.state == State.ASYNC) { - return 1; - } - this.asyncWebRequest.stateLock.unlock(); - return -1; - } - private void releaseLock(int level) { if (level > 0) { this.asyncWebRequest.stateLock.unlock();