diff --git a/android/app/src/main/cpp/iperf_jni.c b/android/app/src/main/cpp/iperf_jni.c index a8f874b..cf4f9ea 100644 --- a/android/app/src/main/cpp/iperf_jni.c +++ b/android/app/src/main/cpp/iperf_jni.c @@ -5,12 +5,25 @@ * the Flutter app to run iperf3 tests natively inside the app process * (no Process.start needed, no SELinux restrictions). * + * THREADING NOTE: + * runIperfLive() BLOCKS for the whole duration of the test. Kotlin must + * call it from a worker thread, never from the Flutter platform thread. + * + * PIPE / SHUTDOWN NOTE: + * iperf's output goes to a pipe drained by a reader thread that blocks in + * fgets(). The reader only sees EOF once the write end is closed, so the + * write end MUST be closed before pthread_join() — joining first deadlocks. + * The state struct is owned solely by the JNI caller, which frees it after + * joining the reader; the reader must never free it. + * * SIGNAL HANDLING NOTE: * iperf3's run() function (called from iperf_run_client()) uses * setjmp/longjmp for graceful signal handling. The JNI bridge must * replicate this setup — otherwise iperf_catch_sigend() installs * handlers that longjmp to an unset buffer, causing undefined - * behaviour (typically a SIGSEGV or hang). + * behaviour (typically a SIGSEGV or hang). Handlers are restored to + * SIG_DFL once the test finishes so a later signal cannot longjmp into + * a stack frame that no longer exists. */ #include @@ -23,6 +36,7 @@ #include #include #include +#include #include "iperf.h" #include "iperf_api.h" @@ -33,31 +47,39 @@ #define LOGI(...) __android_log_print(ANDROID_LOG_INFO, TAG, __VA_ARGS__) #define LOGE(...) __android_log_print(ANDROID_LOG_ERROR, TAG, __VA_ARGS__) -/* Global state for controlling the iperf test */ +/* ------------------------------------------------------------------ */ +/* Global state for controlling the iperf test. */ +/* */ +/* g_test is written by the worker thread running the test and read by */ +/* the platform thread in forceStopIperfTest()/isTestRunning(), so all */ +/* access is serialised through g_test_mutex. The mutex is never held */ +/* across iperf_run_client() — only around pointer publish/retire and */ +/* the (short) stop request. */ +/* ------------------------------------------------------------------ */ static struct iperf_test *g_test = NULL; +static pthread_mutex_t g_test_mutex = PTHREAD_MUTEX_INITIALIZER; static volatile int g_stop = 0; static char g_tmp_template[1024] = "/data/local/tmp/iperf3.XXXXXX"; -/* Per-call state to pass between JNI and the reader thread. +/* Per-call state shared between the JNI caller and the reader thread. * We store the callback object (global ref) and method IDs. * Callbacks are invoked via CallVoidMethod, NOT as function pointers. */ struct iperf_run_state { - JNIEnv *env; jobject callback_ref; /* Global ref to the callback object */ jmethodID mid_output; /* Method ID for onOutput(String) */ jmethodID mid_error; /* Method ID for onError(String) */ jmethodID mid_complete; /* Method ID for onComplete() */ - FILE *outfile; + FILE *outfile; /* Pipe write end; NULL once closed */ int pipe_fd[2]; JavaVM *jvm; pthread_t reader_thread; + int reader_started; /* Only join if pthread_create succeeded */ }; /* Helper: invoke the output callback via CallVoidMethod */ -static void signal_output(struct iperf_run_state *state, const char *msg) { - if (!state || !msg || !msg[0]) return; - JNIEnv *env = state->env; - if (!env || !state->mid_output) return; +static void signal_output(JNIEnv *env, struct iperf_run_state *state, + const char *msg) { + if (!env || !state || !msg || !msg[0] || !state->mid_output) return; jstring jline = (*env)->NewStringUTF(env, msg); if (jline) { @@ -67,10 +89,9 @@ static void signal_output(struct iperf_run_state *state, const char *msg) { } /* Helper: invoke the error callback via CallVoidMethod */ -static void signal_error(struct iperf_run_state *state, const char *msg) { - if (!state || !msg) return; - JNIEnv *env = state->env; - if (!env || !state->mid_error) return; +static void signal_error(JNIEnv *env, struct iperf_run_state *state, + const char *msg) { + if (!env || !state || !msg || !state->mid_error) return; jstring jmsg = (*env)->NewStringUTF(env, msg); if (jmsg) { @@ -80,10 +101,8 @@ static void signal_error(struct iperf_run_state *state, const char *msg) { } /* Helper: invoke the complete callback via CallVoidMethod */ -static void signal_complete(struct iperf_run_state *state) { - if (!state || !state->mid_complete) return; - JNIEnv *env = state->env; - if (!env) return; +static void signal_complete(JNIEnv *env, struct iperf_run_state *state) { + if (!env || !state || !state->mid_complete) return; (*env)->CallVoidMethod(env, state->callback_ref, state->mid_complete); } @@ -91,6 +110,9 @@ static void signal_complete(struct iperf_run_state *state) { /* ------------------------------------------------------------------ */ /* Output reader thread — reads from iperf's output pipe and forwards */ /* each line to the Java callback via JNI. */ +/* */ +/* Exits when the write end of the pipe is closed (EOF). Does NOT own */ +/* `state`: the JNI caller frees it after joining this thread. */ /* ------------------------------------------------------------------ */ static void *reader_thread_func(void *arg) { struct iperf_run_state *state = (struct iperf_run_state *) arg; @@ -101,6 +123,10 @@ static void *reader_thread_func(void *arg) { ret = (*state->jvm)->AttachCurrentThread(state->jvm, &env, NULL); if (ret != JNI_OK) { LOGE("AttachCurrentThread failed: %d", ret); + /* Still drain the pipe so writers never block on a full pipe. */ + char drain[1024]; + while (read(state->pipe_fd[0], drain, sizeof(drain)) > 0) { } + close(state->pipe_fd[0]); return NULL; } @@ -118,49 +144,52 @@ static void *reader_thread_func(void *arg) { (*env)->DeleteLocalRef(env, line); } } - fclose(fp); + fclose(fp); /* also closes pipe_fd[0] */ + } else { + close(state->pipe_fd[0]); } /* Detach this native thread */ (*state->jvm)->DetachCurrentThread(state->jvm); - - /* Free the state struct — caller owns it */ - free(state); return NULL; } /* ------------------------------------------------------------------ */ -/* Convert a Java String[] into a native char** argv array. */ +/* Build a native argv from a Java String[], prepending "iperf3". */ +/* */ +/* iperf_parse_arguments() expects argv[0] to be the program name; */ +/* passing the Dart args directly would make argv[0] "-c", which */ +/* iperf3 misreads as the program name rather than the client flag. */ /* ------------------------------------------------------------------ */ -static char **java_string_array_to_argv(JNIEnv *env, jobjectArray str_array, - int *out_argc) { - int argc = (*env)->GetArrayLength(env, str_array); - if (argc <= 0) { +static char **build_argv(JNIEnv *env, jobjectArray str_array, int *out_argc) { + int user_argc = (*env)->GetArrayLength(env, str_array); + if (user_argc < 0) user_argc = 0; + + int argc = user_argc + 1; /* +1 for argv[0] */ + char **argv = malloc((argc + 1) * sizeof(char *)); + if (!argv) { *out_argc = 0; return NULL; } - char **argv = malloc((argc + 1) * sizeof(char *)); - if (!argv) return NULL; - - for (int i = 0; i < argc; i++) { - jstring jstr = (jstring) (*env)->GetObjectArrayElement(env, - str_array, i); - const char *cstr = (*env)->GetStringUTFChars(env, jstr, NULL); + argv[0] = strdup("iperf3"); + for (int i = 0; i < user_argc; i++) { + jstring jstr = (jstring) (*env)->GetObjectArrayElement(env, str_array, i); + const char *cstr = jstr ? (*env)->GetStringUTFChars(env, jstr, NULL) : NULL; if (cstr) { - argv[i] = strdup(cstr); + argv[i + 1] = strdup(cstr); (*env)->ReleaseStringUTFChars(env, jstr, cstr); } else { - argv[i] = strdup(""); + argv[i + 1] = strdup(""); } - (*env)->DeleteLocalRef(env, jstr); + if (jstr) (*env)->DeleteLocalRef(env, jstr); } argv[argc] = NULL; /* NULL-terminated for iperf */ *out_argc = argc; return argv; } -/* Free an argv array returned by java_string_array_to_argv() */ +/* Free an argv array returned by build_argv() */ static void free_argv(char **argv, int argc) { if (!argv) return; for (int i = 0; i < argc; i++) { @@ -170,42 +199,73 @@ static void free_argv(char **argv, int argc) { } /* ------------------------------------------------------------------ */ -/* Internal: clean up after iperf_run_client returns (normal path). */ -/* Must be called only on the normal return path (NOT after longjmp). */ +/* Close the pipe write end. Idempotent — both the normal and the */ +/* longjmp path call it, and the second call must be a no-op. */ +/* */ +/* Closing this is what makes the reader thread see EOF and exit, so */ +/* it must happen before any pthread_join(). */ +/* ------------------------------------------------------------------ */ +static void close_outfile(struct iperf_run_state *state) { + if (state && state->outfile) { + fflush(state->outfile); + fclose(state->outfile); + state->outfile = NULL; + } +} + +/* Retire the global test pointer and free it, under the mutex. */ +static void retire_test(void) { + pthread_mutex_lock(&g_test_mutex); + struct iperf_test *test = g_test; + g_test = NULL; + if (test) { + /* outfile is a closed FILE* by now — point it somewhere harmless so + * nothing in the teardown path can write to a dead stream. */ + test->outfile = stdout; + iperf_free_test(test); + } + pthread_mutex_unlock(&g_test_mutex); +} + +/* ------------------------------------------------------------------ */ +/* Internal: clean up after iperf_run_client returns and report the */ +/* outcome to Java. Called from both the normal and the longjmp path. */ +/* Frees `state`; do not touch it afterwards. */ /* ------------------------------------------------------------------ */ static void iperf_cleanup_and_signal(JNIEnv *env, struct iperf_run_state *state, char **argv, int argc, int ret, int stopped) { - /* Wait for reader thread to finish */ - pthread_join(state->reader_thread, NULL); + /* Close the write end FIRST: the reader thread is blocked in fgets() + * and only unblocks on EOF. Joining before this would deadlock. */ + close_outfile(state); - /* Flush and close output */ - fflush(state->outfile); - fclose(state->outfile); + if (state->reader_started) { + pthread_join(state->reader_thread, NULL); + state->reader_started = 0; + } - /* Clean up iperf */ - iperf_free_test(g_test); - g_test = NULL; + /* Restore default signal handlers before g_sigend_jmp_buf goes stale. */ + iperf_catch_sigend(SIG_DFL); - /* Free argv */ + retire_test(); free_argv(argv, argc); /* Signal result to Java */ if (stopped) { - signal_output(state, "[iPerf JNI] Test was stopped by user.\n"); + signal_output(env, state, "[iPerf JNI] Test was stopped by user.\n"); } else if (ret < 0) { char errbuf[256]; snprintf(errbuf, sizeof(errbuf), "[iPerf JNI] Test failed: %s\n", iperf_strerror(i_errno)); - signal_error(state, errbuf); + signal_error(env, state, errbuf); } else { - signal_output(state, "[iPerf JNI] Test completed successfully.\n"); + signal_output(env, state, "[iPerf JNI] Test completed successfully.\n"); } /* Signal completion to Java */ - signal_complete(state); + signal_complete(env, state); /* Free global ref and state struct */ (*env)->DeleteGlobalRef(env, state->callback_ref); @@ -228,80 +288,93 @@ static void sigend_handler(int sig) { /* ------------------------------------------------------------------ */ /* JNI: Start an iperf3 test with the given arguments. */ /* Signature: void runIperfLive(String[] args, IperfCallback cb) */ +/* */ +/* BLOCKS until the test finishes — call from a worker thread. */ /* ------------------------------------------------------------------ */ JNIEXPORT void JNICALL Java_com_novoyuuparosk_iperf3gui_IperfRunner_runIperfLive( JNIEnv *env, jobject thiz, jobjectArray arguments, jobject callback) { + /* Resolve callback method IDs up front so failures can be reported. */ + jclass callback_cls = (*env)->GetObjectClass(env, callback); + jmethodID mid_output = (*env)->GetMethodID( + env, callback_cls, "onOutput", "(Ljava/lang/String;)V"); + jmethodID mid_error = (*env)->GetMethodID( + env, callback_cls, "onError", "(Ljava/lang/String;)V"); + jmethodID mid_complete = (*env)->GetMethodID( + env, callback_cls, "onComplete", "()V"); + + /* Report a startup failure directly on the callback object and finish, + * so Dart's completer always resolves instead of waiting for a timeout. */ + #define FAIL_EARLY(msg) do { \ + LOGE("%s", msg); \ + if (mid_error) { \ + jstring jm = (*env)->NewStringUTF(env, msg); \ + if (jm) { \ + (*env)->CallVoidMethod(env, callback, mid_error, jm); \ + (*env)->DeleteLocalRef(env, jm); \ + } \ + } \ + if (mid_complete) (*env)->CallVoidMethod(env, callback, mid_complete); \ + } while (0) + + /* Refuse to start a second concurrent test — the globals below hold + * exactly one test's worth of state. */ + pthread_mutex_lock(&g_test_mutex); + int busy = (g_test != NULL); + pthread_mutex_unlock(&g_test_mutex); + if (busy) { + FAIL_EARLY("[iPerf JNI] A test is already running."); + return; + } + /* Create the iperf test object */ struct iperf_test *test = iperf_new_test(); if (!test) { - const char *err = "Failed to create iperf test"; - LOGE("%s", err); + FAIL_EARLY("[iPerf JNI] Failed to create iperf test"); return; } iperf_defaults(test); - g_test = test; - /* - * Convert Java String[] -> char** argv, prepending "iperf3" as argv[0]. - * iperf_parse_arguments() expects argv[0] to be the program name; - * passing args directly from Dart means argv[0] would be "-c", - * which iperf3 misinterprets as the program name, not the client flag. - */ - int dart_argc = (*env)->GetArrayLength(env, arguments); - int total_argc = dart_argc + 1; /* +1 for "iperf3" */ - char **argv = malloc((total_argc + 1) * sizeof(char *)); + pthread_mutex_lock(&g_test_mutex); + g_test = test; + pthread_mutex_unlock(&g_test_mutex); + + int total_argc = 0; + char **argv = build_argv(env, arguments, &total_argc); if (!argv) { - const char *err = "Failed to allocate argv"; - LOGE("%s", err); - iperf_free_test(test); - g_test = NULL; + retire_test(); + FAIL_EARLY("[iPerf JNI] Failed to allocate argv"); return; } - argv[0] = strdup("iperf3"); - for (int i = 0; i < dart_argc; i++) { - jstring jstr = (jstring) (*env)->GetObjectArrayElement(env, arguments, i); - const char *cstr = (*env)->GetStringUTFChars(env, jstr, NULL); - if (cstr) { - argv[i + 1] = strdup(cstr); - (*env)->ReleaseStringUTFChars(env, jstr, cstr); - } else { - argv[i + 1] = strdup(""); - } - (*env)->DeleteLocalRef(env, jstr); - } - argv[total_argc] = NULL; /* NULL-terminated for iperf */ - /* Set a reasonable connect timeout (30s) to avoid hanging on DNS resolution. - * Default iperf3 connect timeout is 120s which is far too long. */ + /* Set a reasonable connect timeout (30s) to avoid hanging on DNS + * resolution. Default iperf3 connect timeout is 120s, far too long for a + * phone UI. Set before parsing so an explicit --connect-timeout wins. */ test->settings->connect_timeout = 30000; /* Parse arguments */ if (iperf_parse_arguments(test, total_argc, argv) < 0) { char errbuf[256]; snprintf(errbuf, sizeof(errbuf), - "Argument parse error: %s", iperf_strerror(i_errno)); - LOGE("Parse error: %s", errbuf); + "[iPerf JNI] Argument parse error: %s", iperf_strerror(i_errno)); free_argv(argv, total_argc); - iperf_free_test(test); - g_test = NULL; + retire_test(); + FAIL_EARLY(errbuf); return; } - /* Create a pipe to capture iperf's stdout/stderr */ + /* Create a pipe to capture iperf's output */ int pipefd[2]; if (pipe(pipefd) != 0) { + char errbuf[256]; + snprintf(errbuf, sizeof(errbuf), + "[iPerf JNI] Failed to create output pipe: %s", strerror(errno)); free_argv(argv, total_argc); - const char *err = "Failed to create output pipe"; - LOGE("pipe() failed: %s", strerror(errno)); - - signal_error((struct iperf_run_state *) 0, err); - - iperf_free_test(test); - g_test = NULL; + retire_test(); + FAIL_EARLY(errbuf); return; } @@ -312,28 +385,33 @@ Java_com_novoyuuparosk_iperf3gui_IperfRunner_runIperfLive( free_argv(argv, total_argc); close(pipefd[0]); close(pipefd[1]); - iperf_free_test(test); - g_test = NULL; + retire_test(); + FAIL_EARLY("[iPerf JNI] Failed to allocate run state"); return; } - /* Resolve callback method IDs (do this in the calling thread) */ - jclass callback_cls = (*env)->GetObjectClass(env, callback); - state->mid_output = (*env)->GetMethodID( - env, callback_cls, "onOutput", "(Ljava/lang/String;)V"); - state->mid_error = (*env)->GetMethodID( - env, callback_cls, "onError", "(Ljava/lang/String;)V"); - state->mid_complete = (*env)->GetMethodID( - env, callback_cls, "onComplete", "()V"); + FILE *outfile = fdopen(pipefd[1], "w"); + if (!outfile) { + free(state); + free_argv(argv, total_argc); + close(pipefd[0]); + close(pipefd[1]); + retire_test(); + FAIL_EARLY("[iPerf JNI] Failed to open output stream"); + return; + } - /* Get JavaVM for thread attachment */ + /* Get JavaVM for reader-thread attachment */ JavaVM *jvm; (*env)->GetJavaVM(env, &jvm); - state->env = env; + state->mid_output = mid_output; + state->mid_error = mid_error; + state->mid_complete = mid_complete; state->callback_ref = (*env)->NewGlobalRef(env, callback); - state->outfile = fdopen(pipefd[1], "w"); + state->outfile = outfile; state->pipe_fd[0] = pipefd[0]; + state->pipe_fd[1] = pipefd[1]; state->jvm = jvm; /* Redirect iperf's output to our pipe */ @@ -346,42 +424,44 @@ Java_com_novoyuuparosk_iperf3gui_IperfRunner_runIperfLive( * the strdup internally. */ iperf_set_test_template(test, g_tmp_template); + /* Spawn reader thread to forward iperf output to Java. Started before + * the test so early output is not lost, and before setjmp so the + * longjmp path can rely on reader_started being accurate. */ + if (pthread_create(&state->reader_thread, NULL, + reader_thread_func, state) != 0) { + close_outfile(state); + close(pipefd[0]); + free_argv(argv, total_argc); + retire_test(); + (*env)->DeleteGlobalRef(env, state->callback_ref); + free(state); + FAIL_EARLY("[iPerf JNI] Failed to start output reader thread"); + return; + } + state->reader_started = 1; + + #undef FAIL_EARLY + /* * Set up signal handling BEFORE calling iperf_run_client(). * iperf3's run() function uses setjmp/longjmp for signal handling. * - setjmp returns 0 on first call (normal path) * - setjmp returns non-zero when called via longjmp (signal path) * - * Both paths share the same `state`, `argv`, `argc` variables, - * so the stack frame is preserved across the longjmp. + * `state` and `argv` are assigned before setjmp and never reassigned + * afterwards, so both paths observe the same values. */ if (setjmp(g_sigend_jmp_buf) == 0) { /* Normal path — run the test */ signal(SIGPIPE, SIG_IGN); /* Ignore SIGPIPE for socket errors */ iperf_catch_sigend(sigend_handler); - /* Notify Java that the test is starting */ - signal_output(state, "[iPerf JNI] Starting iPerf3 client...\n"); + signal_output(env, state, "[iPerf JNI] Starting iPerf3 client...\n"); - /* Spawn reader thread to forward iperf output to Java */ - if (pthread_create(&state->reader_thread, NULL, - reader_thread_func, state) != 0) { - free_argv(argv, total_argc); - close(pipefd[0]); - close(pipefd[1]); - const char *err = "Failed to start output reader thread"; - LOGE("%s", err); - signal_error(state, err); - iperf_free_test(test); - g_test = NULL; - free(state); - return; - } - - /* Run the test */ g_stop = 0; int ret = iperf_run_client(test); - LOGI("iperf_run_client returned: %d i_errno=%d strerror=%s ctrl_sck=%d", ret, i_errno, iperf_strerror(i_errno), test->ctrl_sck); + LOGI("iperf_run_client returned: %d i_errno=%d strerror=%s", + ret, i_errno, iperf_strerror(i_errno)); /* * We get here after iperf_run_client returns normally. @@ -392,47 +472,43 @@ Java_com_novoyuuparosk_iperf3gui_IperfRunner_runIperfLive( } else { /* * We got here via longjmp() from sigend_handler. - * Clean up resources allocated before the setjmp. - * NOTE: state, argv, argc are still valid on the stack. + * iperf_cleanup_and_signal() closes the pipe, joins the reader and + * frees everything, so there is nothing to unwind by hand here. */ LOGI("Test interrupted by signal %d", g_signed_sig); - - /* Stop flag for cleanup */ g_stop = 1; - - /* Close the output file (the pipe write end) */ - if (state->outfile) { - fclose(state->outfile); - } - - /* Free argv (argv is still valid across longjmp since it's on stack) */ - free_argv(argv, total_argc); - - /* Run the cleanup with stopped=true. - * Note: argv is NULL, argc=0 so free_argv is a no-op. - * The state struct will be freed inside this function. */ - iperf_cleanup_and_signal(env, state, NULL, 0, 0, 1); + iperf_cleanup_and_signal(env, state, argv, total_argc, 0, 1); } } /* ------------------------------------------------------------------ */ /* JNI: Gracefully stop a running iperf3 test. */ /* Signature: void forceStopIperfTest(IperfCallback cb) */ +/* */ +/* Runs on the platform thread while the test runs on a worker thread, */ +/* so the whole body holds g_test_mutex: that keeps the worker from */ +/* freeing g_test underneath us. */ /* ------------------------------------------------------------------ */ JNIEXPORT void JNICALL Java_com_novoyuuparosk_iperf3gui_IperfRunner_forceStopIperfTest( JNIEnv *env, jobject thiz, jobject callback) { - if (!g_test || g_test->done) return; - /* Set the stop flag */ - g_stop = 1; - g_test->done = 1; - iperf_set_send_state(g_test, IPERF_DONE); + pthread_mutex_lock(&g_test_mutex); + int requested = 0; + if (g_test && !g_test->done) { + g_stop = 1; + g_test->done = 1; + iperf_set_send_state(g_test, IPERF_DONE); - /* Try to unblock select() on the control socket */ - if (g_test->ctrl_sck >= 0) { - shutdown(g_test->ctrl_sck, SHUT_RDWR); + /* Try to unblock select() on the control socket */ + if (g_test->ctrl_sck >= 0) { + shutdown(g_test->ctrl_sck, SHUT_RDWR); + } + requested = 1; } + pthread_mutex_unlock(&g_test_mutex); + + if (!requested) return; /* Notify Java */ jclass callback_cls = (*env)->GetObjectClass(env, callback); @@ -440,9 +516,11 @@ Java_com_novoyuuparosk_iperf3gui_IperfRunner_forceStopIperfTest( env, callback_cls, "onOutput", "(Ljava/lang/String;)V"); if (mid_output) { jstring msg = (*env)->NewStringUTF(env, - "[iPerf JNI] Requested graceful stop."); - (*env)->CallVoidMethod(env, callback, mid_output, msg); - (*env)->DeleteLocalRef(env, msg); + "[iPerf JNI] Requested graceful stop.\n"); + if (msg) { + (*env)->CallVoidMethod(env, callback, mid_output, msg); + (*env)->DeleteLocalRef(env, msg); + } } } @@ -453,7 +531,10 @@ Java_com_novoyuuparosk_iperf3gui_IperfRunner_forceStopIperfTest( JNIEXPORT jboolean JNICALL Java_com_novoyuuparosk_iperf3gui_IperfRunner_isTestRunning(JNIEnv *env, jobject thiz) { - return (g_test != NULL && !g_test->done); + pthread_mutex_lock(&g_test_mutex); + jboolean running = (g_test != NULL && !g_test->done); + pthread_mutex_unlock(&g_test_mutex); + return running; } /* ------------------------------------------------------------------ */