From 4173a56b74c8d2202f2ff16cbb8ec9d7f5babafc Mon Sep 17 00:00:00 2001 From: Malcolm Smith Date: Wed, 11 Sep 2024 22:42:51 +0100 Subject: [PATCH 1/2] Switch test_stress_delivery_simultaneous from SIGUSR1 to SIGUSR2 --- Android/testbed/app/src/main/python/main.py | 19 ++++++++++++++++--- Lib/test/test_signal.py | 9 ++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/Android/testbed/app/src/main/python/main.py b/Android/testbed/app/src/main/python/main.py index c7314b500bf821..5a545382067bd4 100644 --- a/Android/testbed/app/src/main/python/main.py +++ b/Android/testbed/app/src/main/python/main.py @@ -5,9 +5,22 @@ import sys # Some tests use SIGUSR1, but that's blocked by default in an Android app in -# order to make it available to `sigwait` in the "Signal Catcher" thread. That -# thread's functionality is only relevant to the JVM ("forcing GC (no HPROF) and -# profile save"), so disabling it should not weaken the tests. +# order to make it available to `sigwait` in the Signal Catcher thread. That +# thread's functionality is only useful for debugging the JVM, so disabling it +# should not weaken the tests. +# +# Simply unblocking SIGUSR1 is enough to fix most tests that use it. But in +# tests that generate multiple different signals in quick succession, it's +# possible for SIGUSR1 to arrive while the main thread is busy running the +# C-level handler for a different signal, in which case the SIGUSR1 may be sent +# to the Signal Catcher thread instead. When this happens, there will be a log +# message containing the text "reacting to signal". +# +# In such cases, the tests may need to be changed. Possible workarounds include: +# * Use a signal other than SIGUSR1 (e.g. test_stress_delivery_simultaneous in +# test_signal.py). +# * Send the signal to a specific thread rather than the whole process (e.g. +# test_signals in test_threadsignals.py. signal.pthread_sigmask(signal.SIG_UNBLOCK, [signal.SIGUSR1]) sys.argv[1:] = shlex.split(os.environ["PYTHON_ARGS"]) diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py index 7f8fe34bb315b2..da36b8576be1ad 100644 --- a/Lib/test/test_signal.py +++ b/Lib/test/test_signal.py @@ -1325,15 +1325,18 @@ def test_stress_delivery_simultaneous(self): def handler(signum, frame): sigs.append(signum) - self.setsig(signal.SIGUSR1, handler) + # On Android, SIGUSR1 is unreliable when used in close proximity to + # another signal – see Android/testbed/app/src/main/python/main.py. + # So we use a different signal. + self.setsig(signal.SIGUSR2, handler) self.setsig(signal.SIGALRM, handler) # for ITIMER_REAL expected_sigs = 0 while expected_sigs < N: # Hopefully the SIGALRM will be received somewhere during - # initial processing of SIGUSR1. + # initial processing of SIGUSR2. signal.setitimer(signal.ITIMER_REAL, 1e-6 + random.random() * 1e-5) - os.kill(os.getpid(), signal.SIGUSR1) + os.kill(os.getpid(), signal.SIGUSR2) expected_sigs += 2 # Wait for handlers to run to avoid signal coalescing From 07175ada697d3a6c93525e3d5a6411383c5c6da8 Mon Sep 17 00:00:00 2001 From: Malcolm Smith Date: Wed, 11 Sep 2024 23:25:14 +0100 Subject: [PATCH 2/2] Improve comment --- Android/testbed/app/src/main/python/main.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Android/testbed/app/src/main/python/main.py b/Android/testbed/app/src/main/python/main.py index 5a545382067bd4..d6941b14412fcc 100644 --- a/Android/testbed/app/src/main/python/main.py +++ b/Android/testbed/app/src/main/python/main.py @@ -5,18 +5,21 @@ import sys # Some tests use SIGUSR1, but that's blocked by default in an Android app in -# order to make it available to `sigwait` in the Signal Catcher thread. That -# thread's functionality is only useful for debugging the JVM, so disabling it -# should not weaken the tests. +# order to make it available to `sigwait` in the Signal Catcher thread. +# (https://cs.android.com/android/platform/superproject/+/android14-qpr3-release:art/runtime/signal_catcher.cc). +# That thread's functionality is only useful for debugging the JVM, so disabling +# it should not weaken the tests. # -# Simply unblocking SIGUSR1 is enough to fix most tests that use it. But in -# tests that generate multiple different signals in quick succession, it's -# possible for SIGUSR1 to arrive while the main thread is busy running the -# C-level handler for a different signal, in which case the SIGUSR1 may be sent -# to the Signal Catcher thread instead. When this happens, there will be a log +# There's no safe way of stopping the thread completely (#123982), but simply +# unblocking SIGUSR1 is enough to fix most tests. +# +# However, in tests that generate multiple different signals in quick +# succession, it's possible for SIGUSR1 to arrive while the main thread is busy +# running the C-level handler for a different signal. In that case, the SIGUSR1 +# may be sent to the Signal Catcher thread instead, which will generate a log # message containing the text "reacting to signal". # -# In such cases, the tests may need to be changed. Possible workarounds include: +# Such tests may need to be changed in one of the following ways: # * Use a signal other than SIGUSR1 (e.g. test_stress_delivery_simultaneous in # test_signal.py). # * Send the signal to a specific thread rather than the whole process (e.g.