From 217812891cd63c20b25379b2cf73f3101416ffe4 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Wed, 10 Feb 2016 16:46:20 -0500 Subject: [PATCH 1/2] Test runner: Avoid race condition A recent extension to the test runner introduced support for running tests in parallel using multi-threading. Following this, the runner would incorrectly emit the "final report" before all individual test results. In order to emit the "final report" at the end of the output stream, the parent thread would initialize all children and wait for availability of a "log lock" shared by all children. According to the documentation on the "threading" module's Lock object [1]: > When more than one thread is blocked in acquire() waiting for the state > to turn to unlocked, only one thread proceeds when a release() call > resets the state to unlocked; which one of the waiting threads proceeds > is not defined, and may vary across implementations. This means the primitive cannot be used by the parent thread to reliably detect completion of all child threads. Update the parent to maintain a reference for each child thread, and to explicitly wait for every child thread to complete before emitting the final result. [1] https://docs.python.org/2/library/threading.html#lock-objects --- tools/packaging/test262.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/packaging/test262.py b/tools/packaging/test262.py index 1192005fc9..26f3f8b526 100755 --- a/tools/packaging/test262.py +++ b/tools/packaging/test262.py @@ -583,6 +583,7 @@ class TestSuite(object): SkipCaseElement.append(SkipElement) TestSuiteElement.append(SkipCaseElement) + threads = [] if workers_count > 1: pool_sem = threading.Semaphore(workers_count) log_lock = threading.Lock() @@ -613,11 +614,13 @@ class TestSuite(object): exec_case() else: pool_sem.acquire() - threading.Thread(target=exec_case).start() + thread = threading.Thread(target=exec_case) + threads.append(thread) + thread.start() pool_sem.release() - if workers_count > 1: - log_lock.acquire() + for thread in threads: + thread.join() if print_summary: self.PrintSummary(progress, logname) @@ -628,9 +631,6 @@ class TestSuite(object): print "Use --full-summary to see output from failed tests" print - if workers_count > 1: - log_lock.release() - return progress.failed def WriteLog(self, result): From b791cc4fbec459b2eef502502e44a3d00688e083 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Wed, 10 Feb 2016 17:15:49 -0500 Subject: [PATCH 2/2] Runner: Re-use lock to share access to stdout When executing multiple tests in parallel, each "child" thread would write to the process's standard output buffer immediately upon test completion. Because thread execution order and instruction interleaving is non-deterministic, this made it possible for characters to be emitted out-of-order. When extended to support multiple concurrent threads, the runner was outfitted with a "log lock" dedicated to sharing access to the output file (when applicable). Re-use this lock when writing to standard out, ensuring proper ordering of test result messages. --- tools/packaging/test262.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/packaging/test262.py b/tools/packaging/test262.py index 26f3f8b526..735d796cac 100755 --- a/tools/packaging/test262.py +++ b/tools/packaging/test262.py @@ -606,10 +606,11 @@ class TestSuite(object): if logname: self.WriteLog(result) finally: + progress.HasRun(result) + if workers_count > 1: log_lock.release() - progress.HasRun(result) if workers_count == 1: exec_case() else: