Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-116622: Kill Android Signal Catcher thread before running tests #123982

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 78 additions & 4 deletions Android/testbed/app/src/main/c/main_activity.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
#include <jni.h>
#include <pthread.h>
#include <Python.h>
#include <signal.h>
#include <stdio.h>
#include <sys/syscall.h>
#include <string.h>
#include <unistd.h>

Expand All @@ -15,6 +17,13 @@ static void throw_runtime_exception(JNIEnv *env, const char *message) {
message);
}

static void throw_errno(JNIEnv *env, char *error_prefix) {
char error_message[1024];
snprintf(error_message, sizeof(error_message),
"%s: %s", error_prefix, strerror(errno));
throw_runtime_exception(env, error_message);
}


// --- Stdio redirection ------------------------------------------------------

Expand Down Expand Up @@ -90,16 +99,81 @@ JNIEXPORT void JNICALL Java_org_python_testbed_PythonTestRunner_redirectStdioToL
for (StreamInfo *si = STREAMS; si->file; si++) {
char *error_prefix;
if ((error_prefix = redirect_stream(si))) {
char error_message[1024];
snprintf(error_message, sizeof(error_message),
"%s: %s", error_prefix, strerror(errno));
throw_runtime_exception(env, error_message);
throw_errno(env, error_prefix);
return;
}
}
}


// --- Signal handling ---------------------------------------------------------

JNIEXPORT void JNICALL Java_org_python_testbed_PythonTestRunner_sendSignal(
JNIEnv *env, jobject obj, int sig
) {
if (kill(getpid(), sig) != 0) {
throw_errno(env, "kill");
return;
}
}

// This signal handler calls the raw _exit system call, which terminates the
// current thread.
static void exit_handler(int sig) {
syscall(SYS_exit, 0);
}

// Android doesn't implement pthread_cancel, but we can achieve something
// similar by forcing the thread to run a signal handler.
JNIEXPORT void JNICALL Java_org_python_testbed_PythonTestRunner_killThread(
JNIEnv *env, jobject obj, int tid
) {
int sig = SIGUSR2;
sighandler_t old_handler;
if ((old_handler = signal(sig, exit_handler)) == SIG_ERR) {
throw_errno(env, "signal (install)");
return;
}
if (tgkill(getpid(), tid, sig) != 0) {
throw_errno(env, "tgkill");
return;
}

// After a short delay, verify that the thread has exited.
usleep(100000);
if (tgkill(getpid(), tid, sig) == 0) {
fprintf(
stderr,
"SignalCatcher TID %d still exists - signal tests may be unreliable",
tid
);
}

if (signal(sig, old_handler) == SIG_ERR) {
throw_errno(env, "signal (uninstall)");
return;
}
}

JNIEXPORT void JNICALL Java_org_python_testbed_PythonTestRunner_unblockSignal(
JNIEnv *env, jobject obj, int sig
) {
sigset_t sigset;
if (sigemptyset(&sigset) != 0) {
throw_errno(env, "sigemptyset");
return;
}
if (sigaddset(&sigset, sig) != 0) {
throw_errno(env, "sigaddset");
return;
}
if (sigprocmask(SIG_UNBLOCK, &sigset, NULL) != 0) {
throw_errno(env, "sigprocmask");
return;
}
}


// --- Python initialization ---------------------------------------------------

static PyStatus set_config_string(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package org.python.testbed

import android.content.Context
import android.os.*
import android.system.Os
import android.system.*
import android.widget.TextView
import androidx.appcompat.app.*
import java.io.*
Expand Down Expand Up @@ -35,6 +35,7 @@ class PythonTestRunner(val context: Context) {
val pythonHome = extractAssets()
System.loadLibrary("main_activity")
redirectStdioToLogcat()
setupSignals()

// The main module is in src/main/python/main.py.
return runPython(pythonHome.toString(), "main")
Expand Down Expand Up @@ -74,6 +75,88 @@ class PythonTestRunner(val context: Context) {
}
}

// Some tests use SIGUSR1, but Android blocks that by default in order to
// make it available to `sigwait` in the SignalCatcher thread
// (https://cs.android.com/android/platform/superproject/+/android14-qpr3-release:art/runtime/signal_catcher.cc).
// That thread is only needed for debugging the JVM, so disabling it should
// not weaken the tests.
//
// Simply unblocking SIGUSR1 is enough to fix simple tests, but in tests
// that involve multiple different signals in quick succession (e.g.
// test_stress_delivery_simultaneous), it's possible for SIGUSR1 to arrive
// while the main thread is running the C-level handler for a different
// signal, in which case the SIGUSR1 may be consumed by the SignalCatcher
// thread instead.
//
// Even if there are other threads with the signal unblocked, it looks like
// these don't have any priority over the `sigwait` – only the main thread
// is special-cased (see `complete_signal` and `do_sigtimedwait` in
// kernel/signal.c). So the only reliable solution is to stop the
// SignalCatcher.
private fun setupSignals() {
val tid = getSignalCatcherTid()
if (tid == 0) {
System.err.println(
"Failed to detect SignalCatcher - signal tests may be unreliable"
)
} else {
// Small delay to make sure the target thread is idle.
Thread.sleep(100);

// This is potentially dangerous, so it's worth always logging here
// in case it causes a deadlock or crash.
System.err.println("Killing SignalCatcher TID $tid")
killThread(tid);
}
unblockSignal(OsConstants.SIGUSR1)
}

// Determine the SignalCatcher's thread ID by sending a signal and waiting
// for it to write a log message.
private fun getSignalCatcherTid() : Int {
sendSignal(OsConstants.SIGUSR1)

val deadline = System.currentTimeMillis() + 1000
try {
while (System.currentTimeMillis() < deadline) {
ProcessBuilder(
// --pid requires API level 24 or higher.
"logcat", "-d", "--pid", Os.getpid().toString()
).start().inputStream.reader().useLines {
var tid = 0;
for (line in it) {
val fields = line.split("""\s+""".toRegex(), 6)
if (fields.size != 6) {
continue
}
if (fields[5].contains("reacting to signal")) {
tid = fields[3].toInt()
}

// SIGUSR1 starts a Java garbage collection, so wait for
// a second message indicating that has completed.
if (
tid != 0 && fields[3].toInt() == tid
&& fields[5].contains("GC freed")
) {
return tid
}
}
}
Thread.sleep(100)
}
} catch (e: IOException) {
// This may happen on ARM64 emulators with API level < 23, where
// SELinux blocks apps from reading their own logs.
e.printStackTrace()
}
return 0;
}

// Native functions are implemented in main_activity.c.
private external fun redirectStdioToLogcat()
private external fun sendSignal(sig: Int)
private external fun killThread(tid: Int)
private external fun unblockSignal(sig: Int)
private external fun runPython(home: String, runModule: String) : Int
}
7 changes: 0 additions & 7 deletions Android/testbed/app/src/main/python/main.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
import os
import runpy
import shlex
import signal
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.
signal.pthread_sigmask(signal.SIG_UNBLOCK, [signal.SIGUSR1])

sys.argv[1:] = shlex.split(os.environ["PYTHON_ARGS"])

# The test module will call sys.exit to indicate whether the tests passed.
Expand Down
Loading