From 561742160beb04dd648a8a02a1ecc851b623b643 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 16 Sep 2020 21:02:09 -0700 Subject: [PATCH 1/4] Add a testcase for #12243 #12244 #12245 [ci skip] --- tests/pthread/test_pthread_proxy_hammer.cpp | 71 +++++++++++++++++++++ tests/test_browser.py | 4 ++ 2 files changed, 75 insertions(+) create mode 100644 tests/pthread/test_pthread_proxy_hammer.cpp diff --git a/tests/pthread/test_pthread_proxy_hammer.cpp b/tests/pthread/test_pthread_proxy_hammer.cpp new file mode 100644 index 0000000000000..a281e76c11fce --- /dev/null +++ b/tests/pthread/test_pthread_proxy_hammer.cpp @@ -0,0 +1,71 @@ +#include +#include +#include +#include +#include +#include + +class random_device +{ + int __f_; +public: + // constructors + explicit random_device(); + ~random_device(); + + // generating functions + unsigned operator()(); +}; + +random_device::random_device() + : __f_(open("/dev/urandom", O_RDONLY)) +{ + if (__f_ < 0) abort(); +} + +random_device::~random_device() +{ + close(__f_); +} + +unsigned +random_device::operator()() +{ + unsigned r; + size_t n = sizeof(r); + char* p = reinterpret_cast(&r); + while (n > 0) + { + ssize_t s = read(__f_, p, 1); + if (s == 0) abort(); + if (s == -1) + { + if (errno != EINTR) abort(); + continue; + } + n -= static_cast(s); + p += static_cast(s); + } + return r; +} + +void* ptr; + +int main() { + int total = 0; + for (int i = 0; i < 128; i++) { + EM_ASM({ out($0, $1) }, i, total); + for (int j = 0; j < 128; j++) { + auto* rd = new random_device; + total += (*rd)(); + ptr = (void*)rd; // make sure the optimizer doesn't remove the allocation + delete rd; + } + } + EM_ASM({ out("done") }); +#ifdef REPORT_RESULT + REPORT_RESULT(0); +#endif + return 0; +} + diff --git a/tests/test_browser.py b/tests/test_browser.py index 39dda6c8807e6..d655a1658ea3b 100644 --- a/tests/test_browser.py +++ b/tests/test_browser.py @@ -4509,6 +4509,10 @@ def run(emcc_args=[]): def test_pthread_reltime(self): self.btest(path_from_root('tests', 'pthread', 'test_pthread_reltime.cpp'), expected='3', args=['-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=1']) + @requires_threads + def test_pthread_proxy_hammer(self): + self.btest(path_from_root('tests', 'pthread', 'test_pthread_proxy_hammer.cpp'), expected='0', args=['-s', 'USE_PTHREADS=1', '-O2', '-s', 'PROXY_TO_PTHREAD']) + # Tests that it is possible to load the main .js file of the application manually via a Blob URL, and still use pthreads. @requires_threads @no_wasm_backend("WASM2JS does not yet support pthreads") From 2f1819936c7cb0b671b37676f121f263952b266e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 17 Sep 2020 08:26:02 -0700 Subject: [PATCH 2/4] better --- tests/pthread/test_pthread_proxy_hammer.cpp | 24 ++++++++++++--------- tests/runner.py | 20 ++++++++--------- tests/test_browser.py | 19 ++++++++++++---- 3 files changed, 39 insertions(+), 24 deletions(-) diff --git a/tests/pthread/test_pthread_proxy_hammer.cpp b/tests/pthread/test_pthread_proxy_hammer.cpp index a281e76c11fce..16cc5be82562d 100644 --- a/tests/pthread/test_pthread_proxy_hammer.cpp +++ b/tests/pthread/test_pthread_proxy_hammer.cpp @@ -18,8 +18,8 @@ class random_device }; random_device::random_device() - : __f_(open("/dev/urandom", O_RDONLY)) { + __f_ = open("/dev/urandom", O_RDONLY); if (__f_ < 0) abort(); } @@ -49,20 +49,24 @@ random_device::operator()() return r; } -void* ptr; - int main() { int total = 0; - for (int i = 0; i < 128; i++) { - EM_ASM({ out($0, $1) }, i, total); - for (int j = 0; j < 128; j++) { - auto* rd = new random_device; - total += (*rd)(); - ptr = (void*)rd; // make sure the optimizer doesn't remove the allocation + for (int i = 0; i < 1024; i++) { + // printf causes proxying + printf("%d %d\n", i, total); + for (int j = 0; j < 1024; j++) { + // allocation uses a mutex + auto* rd = new random_device(); + // reading data causes proxying + for (int k = 0; k < 4; k++) { + total += (*rd)(); + } + // make sure the optimizer doesn't remove the allocation + EM_ASM({ out("iter") }, rd); delete rd; } } - EM_ASM({ out("done") }); + printf("done: %d", total); #ifdef REPORT_RESULT REPORT_RESULT(0); #endif diff --git a/tests/runner.py b/tests/runner.py index 62ef202cbc78e..349237969e983 100755 --- a/tests/runner.py +++ b/tests/runner.py @@ -1364,12 +1364,12 @@ def assert_out_queue_empty(self, who): self.harness_out_queue.get() raise Exception('excessive responses from %s' % who) - # @param tries_left: how many more times to try this test, if it fails. browser tests have - # many more causes of flakiness (in particular, they do not run - # synchronously, so we have a timeout, which can be hit if the VM - # we run on stalls temporarily), so we let each test try more than - # once by default - def run_browser(self, html_file, message, expectedResult=None, timeout=None, tries_left=1): + # @param extra_tries: how many more times to try this test, if it fails. browser tests have + # many more causes of flakiness (in particular, they do not run + # synchronously, so we have a timeout, which can be hit if the VM + # we run on stalls temporarily), so we let each test try more than + # once by default + def run_browser(self, html_file, message, expectedResult=None, timeout=None, extra_tries=1): if not has_browser(): return if BrowserCore.unresponsive_tests >= BrowserCore.MAX_UNRESPONSIVE_TESTS: @@ -1408,10 +1408,10 @@ def run_browser(self, html_file, message, expectedResult=None, timeout=None, tri try: self.assertIdenticalUrlEncoded(expectedResult, output) except Exception as e: - if tries_left > 0: + if extra_tries > 0: print('[test error (see below), automatically retrying]') print(e) - return self.run_browser(html_file, message, expectedResult, timeout, tries_left - 1) + return self.run_browser(html_file, message, expectedResult, timeout, extra_tries - 1) else: raise e finally: @@ -1549,7 +1549,7 @@ def btest(self, filename, expected=None, reference=None, force_c=False, reference_slack=0, manual_reference=False, post_build=None, args=[], outfile='test.html', message='.', also_proxied=False, url_suffix='', timeout=None, also_asmjs=False, - manually_trigger_reftest=False): + manually_trigger_reftest=False, extra_tries=1): assert expected or reference, 'a btest must either expect an output, or have a reference image' # if we are provided the source and not a path, use that filename_is_src = '\n' in filename @@ -1583,7 +1583,7 @@ def btest(self, filename, expected=None, reference=None, force_c=False, post_build() if not isinstance(expected, list): expected = [expected] - self.run_browser(outfile + url_suffix, message, ['/report_result?' + e for e in expected], timeout=timeout) + self.run_browser(outfile + url_suffix, message, ['/report_result?' + e for e in expected], timeout=timeout, extra_tries=extra_tries) # Tests can opt into being run under asmjs as well if 'WASM=0' not in args and (also_asmjs or self.also_asmjs): diff --git a/tests/test_browser.py b/tests/test_browser.py index d655a1658ea3b..059a1a0165b71 100644 --- a/tests/test_browser.py +++ b/tests/test_browser.py @@ -4509,10 +4509,6 @@ def run(emcc_args=[]): def test_pthread_reltime(self): self.btest(path_from_root('tests', 'pthread', 'test_pthread_reltime.cpp'), expected='3', args=['-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=1']) - @requires_threads - def test_pthread_proxy_hammer(self): - self.btest(path_from_root('tests', 'pthread', 'test_pthread_proxy_hammer.cpp'), expected='0', args=['-s', 'USE_PTHREADS=1', '-O2', '-s', 'PROXY_TO_PTHREAD']) - # Tests that it is possible to load the main .js file of the application manually via a Blob URL, and still use pthreads. @requires_threads @no_wasm_backend("WASM2JS does not yet support pthreads") @@ -4915,3 +4911,18 @@ def test_zzz_zzz_4GB_fail(self): # 4GB. self.emcc_args += ['-O2', '-s', 'ALLOW_MEMORY_GROWTH', '-s', 'MAXIMUM_MEMORY=4GB', '-s', 'ABORTING_MALLOC=0'] self.do_run_in_out_file_test('tests', 'browser', 'test_4GB_fail.cpp', js_engines=[V8_ENGINE]) + + #@unittest.skip("only run this manually, to test for race conditions") + @requires_threads + def test_manual_pthread_proxy_hammer(self): + # don't run this with the default extra_tries value, as this is *meant* to + # notice flakes. that is, this looks for a race condition. + # the specific symptom of the hangs that were fixed is that the test hangs + # at some point, using 0% CPU. often that occured in 0-200 iterations out + # of the 1024 the test runs by default, but this can vary by machine and + # browser... + self.btest(path_from_root('tests', 'pthread', 'test_pthread_proxy_hammer.cpp'), + expected='0', + args=['-s', 'USE_PTHREADS=1', '-O2', '-s', 'PROXY_TO_PTHREAD'], + timeout=10000, + extra_tries=0) From 68874d223c8021827172325ab0800ac983caf6c8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 17 Sep 2020 08:26:34 -0700 Subject: [PATCH 3/4] fix --- tests/test_browser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_browser.py b/tests/test_browser.py index 059a1a0165b71..3f3fd2b39f663 100644 --- a/tests/test_browser.py +++ b/tests/test_browser.py @@ -4912,7 +4912,7 @@ def test_zzz_zzz_4GB_fail(self): self.emcc_args += ['-O2', '-s', 'ALLOW_MEMORY_GROWTH', '-s', 'MAXIMUM_MEMORY=4GB', '-s', 'ABORTING_MALLOC=0'] self.do_run_in_out_file_test('tests', 'browser', 'test_4GB_fail.cpp', js_engines=[V8_ENGINE]) - #@unittest.skip("only run this manually, to test for race conditions") + @unittest.skip("only run this manually, to test for race conditions") @requires_threads def test_manual_pthread_proxy_hammer(self): # don't run this with the default extra_tries value, as this is *meant* to From 36be7cd323e41217ee7d58b8873c512fee9c3c9b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 17 Sep 2020 08:27:26 -0700 Subject: [PATCH 4/4] more --- tests/test_browser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_browser.py b/tests/test_browser.py index 3f3fd2b39f663..f3504e96648b2 100644 --- a/tests/test_browser.py +++ b/tests/test_browser.py @@ -4915,8 +4915,6 @@ def test_zzz_zzz_4GB_fail(self): @unittest.skip("only run this manually, to test for race conditions") @requires_threads def test_manual_pthread_proxy_hammer(self): - # don't run this with the default extra_tries value, as this is *meant* to - # notice flakes. that is, this looks for a race condition. # the specific symptom of the hangs that were fixed is that the test hangs # at some point, using 0% CPU. often that occured in 0-200 iterations out # of the 1024 the test runs by default, but this can vary by machine and @@ -4925,4 +4923,6 @@ def test_manual_pthread_proxy_hammer(self): expected='0', args=['-s', 'USE_PTHREADS=1', '-O2', '-s', 'PROXY_TO_PTHREAD'], timeout=10000, + # don't run this with the default extra_tries value, as this is + # *meant* to notice something random, a race condition. extra_tries=0)