From 5fbaa0def334a2d4cc5fda1732b719b1de0d541d Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Thu, 5 Jan 2023 11:25:18 +0100 Subject: [PATCH 1/2] Make `emscripten_check_blocking_allowed()` no-op in release builds This warning should only be raised when linking with `-sASSERTIONS` or `-sALLOW_BLOCKING_ON_MAIN_THREAD=0`, where the latter setting would treat this warning with a higher severity (i.e., causing the program to abort when the main thread is blocked). Previously, this warning was always raised on web environments when not linking with `-sMINIMAL_RUNTIME`. Resolves: #18855. --- src/library_pthread.js | 2 +- test/test_browser.py | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index f3c39e35454d8..9ab596185001f 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -901,7 +901,7 @@ var LibraryPThread = { emscripten_check_blocking_allowed__deps: ['$warnOnce'], emscripten_check_blocking_allowed: function() { -#if ASSERTIONS || !MINIMAL_RUNTIME || !ALLOW_BLOCKING_ON_MAIN_THREAD +#if (ASSERTIONS || !ALLOW_BLOCKING_ON_MAIN_THREAD) && !MINIMAL_RUNTIME #if ENVIRONMENT_MAY_BE_NODE if (ENVIRONMENT_IS_NODE) return; #endif diff --git a/test/test_browser.py b/test/test_browser.py index 2e91a163db1ff..bfa3a0bb8202c 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -3809,14 +3809,23 @@ def test_pthread_main_thread_blocking(self, name): print('Test that we error if not ALLOW_BLOCKING_ON_MAIN_THREAD') self.btest(test_file('pthread/main_thread_%s.cpp' % name), expected='abort:Blocking on the main thread is not allowed by default.', args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) if name == 'join': - print('Test that by default we just warn about blocking on the main thread.') - self.btest_exit(test_file('pthread/main_thread_%s.cpp' % name), args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE']) + create_file('pre.js', ''' + Module['printErr'] = (x) => { + if (x.includes('Blocking on the main thread is very dangerous')) { + maybeReportResultToServer('got_warn'); + } + }; + ''') + print('Test that we warn about blocking on the main thread in debug builds') + self.btest(test_file('pthread/main_thread_join.cpp'), expected='got_warn', args=['-sEXIT_RUNTIME', '-sASSERTIONS', '--pre-js', 'pre.js', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE']) + print('Test that we do not warn about blocking on the main thread in release builds') + self.btest_exit(test_file('pthread/main_thread_join.cpp'), args=['-O3', '--pre-js', 'pre.js', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE']) print('Test that tryjoin is fine, even if not ALLOW_BLOCKING_ON_MAIN_THREAD') self.btest_exit(test_file('pthread/main_thread_join.cpp'), assert_returncode=2, args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-g', '-DTRY_JOIN', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) print('Test that tryjoin is fine, even if not ALLOW_BLOCKING_ON_MAIN_THREAD, and even without a pool') self.btest_exit(test_file('pthread/main_thread_join.cpp'), assert_returncode=2, args=['-O3', '-sUSE_PTHREADS', '-g', '-DTRY_JOIN', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) print('Test that everything works ok when we are on a pthread.') - self.btest_exit(test_file('pthread/main_thread_%s.cpp' % name), args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-sPROXY_TO_PTHREAD', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) + self.btest_exit(test_file('pthread/main_thread_join.cpp'), args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-sPROXY_TO_PTHREAD', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) # Test the old GCC atomic __sync_fetch_and_op builtin operations. @requires_threads From 33468ae7f819b7129882495f59077924b94cf018 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Thu, 2 Mar 2023 10:52:13 +0100 Subject: [PATCH 2/2] Incorporate review comments --- test/test_browser.py | 49 ++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/test/test_browser.py b/test/test_browser.py index bfa3a0bb8202c..a72fad7191b91 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -3800,32 +3800,31 @@ def test_pthread_64bit_cxx11_atomics(self, opt): def test_pthread_hardware_concurrency(self): self.btest_exit(test_file('pthread/test_pthread_hardware_concurrency.cpp'), args=['-O2', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE="navigator.hardwareConcurrency"']) - @parameterized({ - 'join': ('join',), - 'wait': ('wait',), - }) + # Test that we error if not ALLOW_BLOCKING_ON_MAIN_THREAD @requires_threads - def test_pthread_main_thread_blocking(self, name): - print('Test that we error if not ALLOW_BLOCKING_ON_MAIN_THREAD') - self.btest(test_file('pthread/main_thread_%s.cpp' % name), expected='abort:Blocking on the main thread is not allowed by default.', args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) - if name == 'join': - create_file('pre.js', ''' - Module['printErr'] = (x) => { - if (x.includes('Blocking on the main thread is very dangerous')) { - maybeReportResultToServer('got_warn'); - } - }; - ''') - print('Test that we warn about blocking on the main thread in debug builds') - self.btest(test_file('pthread/main_thread_join.cpp'), expected='got_warn', args=['-sEXIT_RUNTIME', '-sASSERTIONS', '--pre-js', 'pre.js', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE']) - print('Test that we do not warn about blocking on the main thread in release builds') - self.btest_exit(test_file('pthread/main_thread_join.cpp'), args=['-O3', '--pre-js', 'pre.js', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE']) - print('Test that tryjoin is fine, even if not ALLOW_BLOCKING_ON_MAIN_THREAD') - self.btest_exit(test_file('pthread/main_thread_join.cpp'), assert_returncode=2, args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-g', '-DTRY_JOIN', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) - print('Test that tryjoin is fine, even if not ALLOW_BLOCKING_ON_MAIN_THREAD, and even without a pool') - self.btest_exit(test_file('pthread/main_thread_join.cpp'), assert_returncode=2, args=['-O3', '-sUSE_PTHREADS', '-g', '-DTRY_JOIN', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) - print('Test that everything works ok when we are on a pthread.') - self.btest_exit(test_file('pthread/main_thread_join.cpp'), args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-sPROXY_TO_PTHREAD', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) + def test_pthread_main_thread_blocking_wait(self): + self.btest(test_file('pthread/main_thread_wait.cpp'), expected='abort:Blocking on the main thread is not allowed by default.', args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) + + # Test that we error or warn depending on ALLOW_BLOCKING_ON_MAIN_THREAD or ASSERTIONS + @requires_threads + def test_pthread_main_thread_blocking_join(self): + create_file('pre.js', ''' + Module['printErr'] = (x) => { + if (x.includes('Blocking on the main thread is very dangerous')) { + maybeReportResultToServer('got_warn'); + } + }; + ''') + # Test that we warn about blocking on the main thread in debug builds + self.btest(test_file('pthread/main_thread_join.cpp'), expected='got_warn', args=['-sEXIT_RUNTIME', '-sASSERTIONS', '--pre-js', 'pre.js', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE']) + # Test that we do not warn about blocking on the main thread in release builds + self.btest_exit(test_file('pthread/main_thread_join.cpp'), args=['-O3', '--pre-js', 'pre.js', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE']) + # Test that tryjoin is fine, even if not ALLOW_BLOCKING_ON_MAIN_THREAD + self.btest_exit(test_file('pthread/main_thread_join.cpp'), assert_returncode=2, args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-g', '-DTRY_JOIN', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) + # Test that tryjoin is fine, even if not ALLOW_BLOCKING_ON_MAIN_THREAD, and even without a pool + self.btest_exit(test_file('pthread/main_thread_join.cpp'), assert_returncode=2, args=['-O3', '-sUSE_PTHREADS', '-g', '-DTRY_JOIN', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) + # Test that everything works ok when we are on a pthread + self.btest_exit(test_file('pthread/main_thread_join.cpp'), args=['-O3', '-sUSE_PTHREADS', '-sPTHREAD_POOL_SIZE', '-sPROXY_TO_PTHREAD', '-sALLOW_BLOCKING_ON_MAIN_THREAD=0']) # Test the old GCC atomic __sync_fetch_and_op builtin operations. @requires_threads