From 2104252244206ca6788e1f07fda0c19a61c9839f Mon Sep 17 00:00:00 2001 From: Fredrik Fornwall Date: Wed, 8 Jun 2016 01:37:08 +0200 Subject: [PATCH] Change session exit detection Previously we waited for all opened file descriptors to the terminal to be closed. This caused problem when e.g. running "sleep 900 &" and then exiting the shell, with sleep keeping the session alive and had to be killed manually (killing the process group did not help - the shell had already exited and was in zombie state). This is also what most other terminal emulators do. Relatedly, switch to sending SIGKILL to force quit a session instead of SIGHUP, since SIGHUP can be ignored. --- .../main/java/com/termux/terminal/JNI.java | 9 ---- .../com/termux/terminal/TerminalSession.java | 52 ++++++++++--------- app/src/main/jni/termux.c | 5 -- 3 files changed, 28 insertions(+), 38 deletions(-) diff --git a/app/src/main/java/com/termux/terminal/JNI.java b/app/src/main/java/com/termux/terminal/JNI.java index c0cff30a..ed3bc570 100644 --- a/app/src/main/java/com/termux/terminal/JNI.java +++ b/app/src/main/java/com/termux/terminal/JNI.java @@ -40,15 +40,6 @@ final class JNI { */ public static native int waitFor(int processId); - /** - * Send SIGHUP to a process group. - * - * There exists a kill(2) system call wrapper in {@link android.os.Process#sendSignal(int, int)}, but that makes a - * "if (pid > 0)" check so cannot be used for sending to a process group: - * https://android.googlesource.com/platform/frameworks/base/+/donut-release/core/jni/android_util_Process.cpp - */ - public static native void hangupProcessGroup(int processId); - /** Close a file descriptor through the close(2) system call. */ public static native void close(int fileDescriptor); diff --git a/app/src/main/java/com/termux/terminal/TerminalSession.java b/app/src/main/java/com/termux/terminal/TerminalSession.java index 979e7798..c7ecec3b 100644 --- a/app/src/main/java/com/termux/terminal/TerminalSession.java +++ b/app/src/main/java/com/termux/terminal/TerminalSession.java @@ -1,5 +1,13 @@ package com.termux.terminal; +import android.annotation.SuppressLint; +import android.os.Handler; +import android.os.Message; +import android.system.ErrnoException; +import android.system.Os; +import android.system.OsConstants; +import android.util.Log; + import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.FileOutputStream; @@ -9,11 +17,6 @@ import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.util.UUID; -import android.annotation.SuppressLint; -import android.os.Handler; -import android.os.Message; -import android.util.Log; - /** * A terminal session, consisting of a process coupled to a terminal interface. *

@@ -190,10 +193,6 @@ public final class TerminalSession extends TerminalOutput { } } catch (Exception e) { // Ignore, just shutting down. - } finally { - // Now wait for process exit: - int processExitCode = JNI.waitFor(mShellPid); - mMainThreadHandler.sendMessage(mMainThreadHandler.obtainMessage(MSG_PROCESS_EXITED, processExitCode)); } } }.start(); @@ -213,7 +212,16 @@ public final class TerminalSession extends TerminalOutput { } } }.start(); - } + + new Thread("TermSessionWaiter[pid=" + mShellPid + "]") { + @Override + public void run() { + int processExitCode = JNI.waitFor(mShellPid); + mMainThreadHandler.sendMessage(mMainThreadHandler.obtainMessage(MSG_PROCESS_EXITED, processExitCode)); + } + }.start(); + + } /** Write data to the shell process. */ @Override @@ -273,20 +281,16 @@ public final class TerminalSession extends TerminalOutput { notifyScreenUpdate(); } - /** - * Finish this terminal session. Frees resources used by the terminal emulator and closes the attached - * InputStream and OutputStream. - */ - public void finishIfRunning() { - if (isRunning()) { - JNI.hangupProcessGroup(mShellPid); - // Stop the reader and writer threads, and close the I/O streams. Note that - // cleanupResources() will be run later. - mTerminalToProcessIOQueue.close(); - mProcessToTerminalIOQueue.close(); - JNI.close(mTerminalFileDescriptor); - } - } + /** Finish this terminal session by sending SIGKILL to the shell. */ + public void finishIfRunning() { + if (isRunning()) { + try { + Os.kill(mShellPid, OsConstants.SIGKILL); + } catch (ErrnoException e) { + Log.w("termux", "Failed sending SIGKILL: " + e.getMessage()); + } + } + } /** Cleanup resources when the process exits. */ void cleanupResources(int exitStatus) { diff --git a/app/src/main/jni/termux.c b/app/src/main/jni/termux.c index fcc81891..cb1c9dec 100644 --- a/app/src/main/jni/termux.c +++ b/app/src/main/jni/termux.c @@ -208,11 +208,6 @@ JNIEXPORT int JNICALL Java_com_termux_terminal_JNI_waitFor(JNIEnv* TERMUX_UNUSED } } -JNIEXPORT void JNICALL Java_com_termux_terminal_JNI_hangupProcessGroup(JNIEnv* TERMUX_UNUSED(env), jclass TERMUX_UNUSED(clazz), jint procId) -{ - killpg(procId, SIGHUP); -} - JNIEXPORT void JNICALL Java_com_termux_terminal_JNI_close(JNIEnv* TERMUX_UNUSED(env), jclass TERMUX_UNUSED(clazz), jint fileDescriptor) { close(fileDescriptor);