This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 7.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 87dbf83dd51616cbca6b5ee7406684ebcade1c4e Author: Mark Thomas <[email protected]> AuthorDate: Tue Oct 15 23:11:59 2019 +0100 Refactor the unit test to avoid race conditions If an I/O error occurs on a non-container thread that will trigger a container thread to process the error - primarily to fire on onError() event. If the app tries to handle the error on the non-container thread a race condition is very likely. --- .../core/TestAsyncContextStateChanges.java | 100 +++++++++++++++------ 1 file changed, 72 insertions(+), 28 deletions(-) diff --git a/test/org/apache/catalina/core/TestAsyncContextStateChanges.java b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java index 5613c07..5c2001f 100644 --- a/test/org/apache/catalina/core/TestAsyncContextStateChanges.java +++ b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java @@ -160,7 +160,11 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest { servletRequest = req; asyncContext = req.startAsync(); asyncContext.addListener(new Listener()); - asyncContext.setTimeout(1000); + if (!asyncEnd.isError()) { + // Use a short timeout so the test does not pause for too long + // waiting for the timeout to be triggered. + asyncContext.setTimeout(1000); + } Thread t = new NonContainerThread(); switch (endTiming) { @@ -231,35 +235,35 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest { } } - try { - switch (asyncEnd) { - case COMPLETE: - case ERROR_COMPLETE: { - asyncContext.complete(); - break; + if (endTiming != EndTiming.THREAD_AFTER_EXIT) { + try { + switch (asyncEnd) { + case COMPLETE: + case ERROR_COMPLETE: { + asyncContext.complete(); + break; + } + case DISPATCH: + case ERROR_DISPATCH: { + dispatch = true; + asyncContext.dispatch(); + break; + } + case NONE: + case ERROR_NONE: { + break; + } } - case DISPATCH: - case ERROR_DISPATCH: { - dispatch = true; - asyncContext.dispatch(); - break; + + // The request must stay in async mode until doGet() exists + if (servletRequest.isAsyncStarted()) { + failed.set(false); } - case NONE: - case ERROR_NONE: { - break; + } finally { + if (endTiming == EndTiming.THREAD_BEFORE_EXIT) { + threadLatch.countDown(); } } - - // The request must stay in async mode until doGet() exists - if (servletRequest.isAsyncStarted() != (endTiming == EndTiming.THREAD_AFTER_EXIT && !asyncEnd.isNone())) { - failed.set(false); - } - } finally { - if (endTiming == EndTiming.THREAD_BEFORE_EXIT) { - threadLatch.countDown(); - } else if (endTiming == EndTiming.THREAD_AFTER_EXIT) { - endLatch.countDown(); - } } } } @@ -276,12 +280,52 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest { @Override public void onTimeout(AsyncEvent event) throws IOException { - // NO-OP + // Need to handle timeouts for THREAD_AFTER_EXIT in the listener to + // avoid concurrency issues. + if (endTiming == EndTiming.THREAD_AFTER_EXIT) { + switch (asyncEnd) { + case COMPLETE: { + asyncContext.complete(); + break; + } + case DISPATCH: { + dispatch = true; + asyncContext.dispatch(); + break; + } + default: + // NO-OP + } + } + if (servletRequest.isAsyncStarted() == asyncEnd.isNone()) { + failed.set(false); + } + endLatch.countDown(); } @Override public void onError(AsyncEvent event) throws IOException { - // NO-OP + // Need to handle errors for THREAD_AFTER_EXIT in the listener to + // avoid concurrency issues. + if (endTiming == EndTiming.THREAD_AFTER_EXIT) { + switch (asyncEnd) { + case ERROR_COMPLETE: { + asyncContext.complete(); + break; + } + case ERROR_DISPATCH: { + dispatch = true; + asyncContext.dispatch(); + break; + } + default: + // NO-OP + } + if (servletRequest.isAsyncStarted() == asyncEnd.isNone()) { + failed.set(false); + } + endLatch.countDown(); + } } @Override --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
