CronetUrlRequestContextTest.testShutdown() not always executed to the end |
|||
Issue description
There is a race condition between the thread that executes the test and the executor thread that handles Cronet callbacks. If the test thread wins, the shutdown operation may not be executed.
ShutdownTestUrlRequestCallback.onSuccess() executes the following logic:
public void onSucceeded(UrlRequest request, UrlResponseInfo info) {
super.onSucceeded(request, info);
mTestFramework.mCronetEngine.shutdown();
}
Internally, super.onSucceeded() opens ConditionVariable that blocks the test thread. As soon as the condition opens, the test finishes. This will result in immediate termination of all other running thread. As the result, mTestFramework.mCronetEngine.shutdown() may not be executed at all, or it can be terminated somewhere in the middle of the execution.
,
Aug 17 2016
,
Aug 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d5bef478ace1273acb57a4342be9558e0b7daaa commit 1d5bef478ace1273acb57a4342be9558e0b7daaa Author: kapishnikov <kapishnikov@chromium.org> Date: Thu Aug 18 17:46:57 2016 Fix flaky tests in CronetUrlRequestContextTest 1. Fix of Bug 635025 & 596929. When CronetUrlRequestContextTest#testShutdown() is run multiple times, a race condition is created between the test thread and the executor threads. When the second run starts, the test replaces the value of mTestFramework with a new CronetEngine value; however, the executor thread may still be executing ShutdownTestUrlRequestCallback from the previous run, which also references the same shared mTestFramework. If the test thread replaces the value of mTestFramework while onSucceeded() is still being executed, the callback may shut down the wrong engine. The same reasoning applies to testShutdownAfterError(). The reason why the test was executed multiple times is due to http://crbug/637972. 2. Fix of Bug 637986 . Block the test thread until the request callback finishes its work. See changes in ShutdownTestUrlRequestCallback#mCallbackCompletionBlock 3. At the end of the test, shut down the callback executor, to release the underlying thread. 4. Removed mTestFramework member variable to avoid similar issues in the future. BUG= 596929 , 635025 , 637986 Review-Url: https://codereview.chromium.org/2254043002 Cr-Commit-Position: refs/heads/master@{#412885} [modify] https://crrev.com/1d5bef478ace1273acb57a4342be9558e0b7daaa/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java
,
Aug 18 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by pauljensen@chromium.org
, Aug 16 2016