You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/10/15 23:16:54 UTC

[tomcat] 07/07: Refactor the unit test to avoid race conditions

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 <ma...@apache.org>
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org