From e3c85984622bc8e8eae2dbbbfe467691ff91501d Mon Sep 17 00:00:00 2001 From: Christian Landvogt Date: Sun, 28 Jan 2024 21:58:49 +0100 Subject: [PATCH 1/5] Add ACTION_SERVICE_STOP intent to only stop a single AppShell --- .../java/com/termux/app/TermuxService.java | 22 +++++++++ .../shell/command/runner/app/AppShell.java | 46 +++++++++++++++++++ .../termux/shared/termux/TermuxConstants.java | 6 +++ 3 files changed, 74 insertions(+) diff --git a/app/src/main/java/com/termux/app/TermuxService.java b/app/src/main/java/com/termux/app/TermuxService.java index 8025d0bd2c..e512ca6b00 100644 --- a/app/src/main/java/com/termux/app/TermuxService.java +++ b/app/src/main/java/com/termux/app/TermuxService.java @@ -8,6 +8,7 @@ import android.content.Context; import android.content.Intent; import android.content.res.Resources; +import android.net.Uri; import android.net.wifi.WifiManager; import android.os.Binder; import android.os.Build; @@ -154,6 +155,10 @@ public int onStartCommand(Intent intent, int flags, int startId) { Logger.logDebug(LOG_TAG, "ACTION_SERVICE_EXECUTE intent received"); actionServiceExecute(intent); break; + case TERMUX_SERVICE.ACTION_SERVICE_STOP: + Logger.logDebug(LOG_TAG, "ACTION_SERVICE_STOP intent received"); + actionServiceStop(intent); + break; default: Logger.logError(LOG_TAG, "Invalid action: \"" + action + "\""); break; @@ -354,6 +359,23 @@ private void actionReleaseWakeLock(boolean updateNotification) { Logger.logDebug(LOG_TAG, "WakeLocks released successfully"); } + private void actionServiceStop(Intent intent) { + if (intent == null) { + Logger.logError(LOG_TAG, "Ignoring null intent to actionServiceStop"); + return; + } + + int gracePeriod = IntentUtils.getIntegerExtraIfSet(intent, TERMUX_SERVICE.EXTRA_TERMINATE_GRACE_PERIOD, 5000); + + Uri executableUri = intent.getData(); + String executable = UriUtils.getUriFilePathWithFragment(executableUri); + String shellName = ShellUtils.getExecutableBasename(executable); + AppShell appShell = getTermuxTaskForShellName(shellName); + if (appShell != null) { + appShell.terminateIfExecuting(getApplicationContext(), gracePeriod, true); + } + } + /** Process {@link TERMUX_SERVICE#ACTION_SERVICE_EXECUTE} intent to execute a shell command in * a foreground TermuxSession or in a background TermuxTask. */ private void actionServiceExecute(Intent intent) { diff --git a/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java b/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java index 3f49e5aebb..f2d15fd106 100644 --- a/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java +++ b/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java @@ -1,6 +1,7 @@ package com.termux.shared.shell.command.runner.app; import android.content.Context; +import android.os.Handler; import android.system.ErrnoException; import android.system.Os; import android.system.OsConstants; @@ -245,6 +246,37 @@ private void executeInner(@NonNull final Context context) throws IllegalThreadSt AppShell.processAppShellResult(this, null); } + /** + * Terminate this {@link AppShell} by sending a {@link OsConstants#SIGTERM} to its {@link #mProcess} + * if it is still executing. After {@code gracePeriodMsec} milliseconds {@link OsConstants#SIGTERM} is + * signalled. + * + * @param context The {@link Context} for operations. + * @param gracePeriodMsec The delay after which a SIGKILL is send. + * @param processResult If set to {@code true}, then the {@link #processAppShellResult(AppShell, ExecutionCommand)} + * will be called to process the failure. + */ + public void terminateIfExecuting(@NonNull final Context context, long gracePeriodMsec, boolean processResult) { + if (gracePeriodMsec == 0) { + killIfExecuting(context, processResult); + return; + } + + // If execution command has already finished executing, then no need to process results or sending any signals + if (mExecutionCommand.hasExecuted()) { + Logger.logDebug(LOG_TAG, "Ignoring sending SIGTERM or SIGKILL to \"" + mExecutionCommand.getCommandIdAndLabelLogString() + "\" AppShell since it has already finished executing"); + return; + } + + Logger.logDebug(LOG_TAG, "Send SIGTERM to \"" + mExecutionCommand.getCommandIdAndLabelLogString() + "\" AppShell"); + + if (mExecutionCommand.isExecuting()) { + term(); + } + + (new Handler()).postDelayed(() -> killIfExecuting(context, processResult), gracePeriodMsec); + } + /** * Kill this {@link AppShell} by sending a {@link OsConstants#SIGILL} to its {@link #mProcess} * if its still executing. @@ -274,6 +306,20 @@ public void killIfExecuting(@NonNull final Context context, boolean processResul } } + + /** + * Terminate this {@link AppShell} by sending a {@link OsConstants#SIGTERM} to its {@link #mProcess}. + */ + public void term() { + int pid = ShellUtils.getPid(mProcess); + try { + // Send SIGKILL to process + Os.kill(pid, OsConstants.SIGTERM); + } catch (ErrnoException e) { + Logger.logWarn(LOG_TAG, "Failed to send SIGTERM to \"" + mExecutionCommand.getCommandIdAndLabelLogString() + "\" AppShell with pid " + pid + ": " + e.getMessage()); + } + } + /** * Kill this {@link AppShell} by sending a {@link OsConstants#SIGILL} to its {@link #mProcess}. */ diff --git a/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java b/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java index 59f523af72..21029ccafa 100644 --- a/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java +++ b/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java @@ -993,6 +993,9 @@ public static final class TERMUX_SERVICE { /** Intent action to execute command with TERMUX_SERVICE */ public static final String ACTION_SERVICE_EXECUTE = TERMUX_PACKAGE_NAME + ".service_execute"; // Default: "com.termux.service_execute" + /** Intent action to execute command with TERMUX_SERVICE */ + public static final String ACTION_SERVICE_STOP = TERMUX_PACKAGE_NAME + ".service_execution_stop"; // Default: "com.termux.service_execute" + /** Uri scheme for paths sent via intent to TERMUX_SERVICE */ public static final String URI_SCHEME_SERVICE_EXECUTE = TERMUX_PACKAGE_NAME + ".file"; // Default: "com.termux.file" /** Intent {@code String[]} extra for arguments to the executable of the command for the TERMUX_SERVICE.ACTION_SERVICE_EXECUTE intent */ @@ -1046,6 +1049,9 @@ public static final class TERMUX_SERVICE { * be created in {@link #EXTRA_RESULT_DIRECTORY} if {@link #EXTRA_RESULT_SINGLE_FILE} is * {@code false} for the TERMUX_SERVICE.ACTION_SERVICE_EXECUTE intent */ public static final String EXTRA_RESULT_FILES_SUFFIX = TERMUX_PACKAGE_NAME + ".execute.result_files_suffix"; // Default: "com.termux.execute.result_files_suffix" + /** Intent {@code long} extra for graceperiod between SIGTERM and SIGKILL + * for the TERMUX_SERVICE.ACTION_SERVICE_STOP intent */ + public static final String EXTRA_TERMINATE_GRACE_PERIOD = TERMUX_PACKAGE_NAME + ".execute.stop.delay"; From e5299dbb9ba8937b128e814bfabb9aaa2337d1d8 Mon Sep 17 00:00:00 2001 From: Christian Landvogt Date: Mon, 5 Feb 2024 22:12:32 +0100 Subject: [PATCH 2/5] Fix review findings --- app/src/main/java/com/termux/app/TermuxService.java | 11 ++++++----- .../com/termux/shared/termux/TermuxConstants.java | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/termux/app/TermuxService.java b/app/src/main/java/com/termux/app/TermuxService.java index e512ca6b00..b1fe5e6460 100644 --- a/app/src/main/java/com/termux/app/TermuxService.java +++ b/app/src/main/java/com/termux/app/TermuxService.java @@ -8,7 +8,6 @@ import android.content.Context; import android.content.Intent; import android.content.res.Resources; -import android.net.Uri; import android.net.wifi.WifiManager; import android.os.Binder; import android.os.Build; @@ -365,11 +364,13 @@ private void actionServiceStop(Intent intent) { return; } - int gracePeriod = IntentUtils.getIntegerExtraIfSet(intent, TERMUX_SERVICE.EXTRA_TERMINATE_GRACE_PERIOD, 5000); + String shellName = IntentUtils.getStringExtraIfSet(intent, TERMUX_SERVICE.EXTRA_SHELL_NAME, null); + if (shellName == null) { + Logger.logError(LOG_TAG, "Ignoring intent since it did not contain explicit shell name"); + return; + } - Uri executableUri = intent.getData(); - String executable = UriUtils.getUriFilePathWithFragment(executableUri); - String shellName = ShellUtils.getExecutableBasename(executable); + int gracePeriod = IntentUtils.getIntegerExtraIfSet(intent, TERMUX_SERVICE.EXTRA_TERMINATE_GRACE_PERIOD, 5000); AppShell appShell = getTermuxTaskForShellName(shellName); if (appShell != null) { appShell.terminateIfExecuting(getApplicationContext(), gracePeriod, true); diff --git a/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java b/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java index 21029ccafa..fb15c24367 100644 --- a/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java +++ b/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java @@ -993,8 +993,8 @@ public static final class TERMUX_SERVICE { /** Intent action to execute command with TERMUX_SERVICE */ public static final String ACTION_SERVICE_EXECUTE = TERMUX_PACKAGE_NAME + ".service_execute"; // Default: "com.termux.service_execute" - /** Intent action to execute command with TERMUX_SERVICE */ - public static final String ACTION_SERVICE_STOP = TERMUX_PACKAGE_NAME + ".service_execution_stop"; // Default: "com.termux.service_execute" + /** Intent action to stop command execution with TERMUX_SERVICE */ + public static final String ACTION_SERVICE_STOP = TERMUX_PACKAGE_NAME + ".service_execution_stop"; // Default: "com.termux.service_execution_stop" /** Uri scheme for paths sent via intent to TERMUX_SERVICE */ public static final String URI_SCHEME_SERVICE_EXECUTE = TERMUX_PACKAGE_NAME + ".file"; // Default: "com.termux.file" @@ -1051,7 +1051,7 @@ public static final class TERMUX_SERVICE { public static final String EXTRA_RESULT_FILES_SUFFIX = TERMUX_PACKAGE_NAME + ".execute.result_files_suffix"; // Default: "com.termux.execute.result_files_suffix" /** Intent {@code long} extra for graceperiod between SIGTERM and SIGKILL * for the TERMUX_SERVICE.ACTION_SERVICE_STOP intent */ - public static final String EXTRA_TERMINATE_GRACE_PERIOD = TERMUX_PACKAGE_NAME + ".execute.stop.delay"; + public static final String EXTRA_TERMINATE_GRACE_PERIOD = TERMUX_PACKAGE_NAME + ".execute.stop.delay"; // Default: "com.termux.execute.stop.delay" From 81fb1c16707d99c772eba9435c7e231ba08af7e1 Mon Sep 17 00:00:00 2001 From: Christian Landvogt Date: Fri, 9 Feb 2024 20:10:23 +0100 Subject: [PATCH 3/5] Fix review findings #2 * fix stop_delay constant * terminate all appshells with matching name --- app/src/main/java/com/termux/app/TermuxService.java | 3 ++- .../main/java/com/termux/shared/termux/TermuxConstants.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/termux/app/TermuxService.java b/app/src/main/java/com/termux/app/TermuxService.java index b1fe5e6460..8534f84fab 100644 --- a/app/src/main/java/com/termux/app/TermuxService.java +++ b/app/src/main/java/com/termux/app/TermuxService.java @@ -372,8 +372,9 @@ private void actionServiceStop(Intent intent) { int gracePeriod = IntentUtils.getIntegerExtraIfSet(intent, TERMUX_SERVICE.EXTRA_TERMINATE_GRACE_PERIOD, 5000); AppShell appShell = getTermuxTaskForShellName(shellName); - if (appShell != null) { + while (appShell != null) { appShell.terminateIfExecuting(getApplicationContext(), gracePeriod, true); + appShell = getTermuxTaskForShellName(shellName); } } diff --git a/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java b/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java index fb15c24367..df3c311fc7 100644 --- a/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java +++ b/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java @@ -1051,7 +1051,7 @@ public static final class TERMUX_SERVICE { public static final String EXTRA_RESULT_FILES_SUFFIX = TERMUX_PACKAGE_NAME + ".execute.result_files_suffix"; // Default: "com.termux.execute.result_files_suffix" /** Intent {@code long} extra for graceperiod between SIGTERM and SIGKILL * for the TERMUX_SERVICE.ACTION_SERVICE_STOP intent */ - public static final String EXTRA_TERMINATE_GRACE_PERIOD = TERMUX_PACKAGE_NAME + ".execute.stop.delay"; // Default: "com.termux.execute.stop.delay" + public static final String EXTRA_TERMINATE_GRACE_PERIOD = TERMUX_PACKAGE_NAME + ".execute.stop_delay"; // Default: "com.termux.execute.stop_delay" From caf2100161d21d50fd97518f026f5f359651cd00 Mon Sep 17 00:00:00 2001 From: Christian Landvogt Date: Fri, 9 Feb 2024 20:12:38 +0100 Subject: [PATCH 4/5] Fix comment --- .../com/termux/shared/shell/command/runner/app/AppShell.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java b/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java index f2d15fd106..3592873e32 100644 --- a/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java +++ b/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java @@ -313,7 +313,7 @@ public void killIfExecuting(@NonNull final Context context, boolean processResul public void term() { int pid = ShellUtils.getPid(mProcess); try { - // Send SIGKILL to process + // Send SIGTERM to process Os.kill(pid, OsConstants.SIGTERM); } catch (ErrnoException e) { Logger.logWarn(LOG_TAG, "Failed to send SIGTERM to \"" + mExecutionCommand.getCommandIdAndLabelLogString() + "\" AppShell with pid " + pid + ": " + e.getMessage()); From 7d3ff3ea1121b4e3869994df21c1275c777515e2 Mon Sep 17 00:00:00 2001 From: Christian Landvogt Date: Sun, 11 Feb 2024 20:46:48 +0100 Subject: [PATCH 5/5] Rename constant --- app/src/main/java/com/termux/app/TermuxService.java | 4 ++-- .../shared/shell/command/runner/app/AppShell.java | 10 +++++----- .../java/com/termux/shared/termux/TermuxConstants.java | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/termux/app/TermuxService.java b/app/src/main/java/com/termux/app/TermuxService.java index 8534f84fab..da5fdc471b 100644 --- a/app/src/main/java/com/termux/app/TermuxService.java +++ b/app/src/main/java/com/termux/app/TermuxService.java @@ -370,10 +370,10 @@ private void actionServiceStop(Intent intent) { return; } - int gracePeriod = IntentUtils.getIntegerExtraIfSet(intent, TERMUX_SERVICE.EXTRA_TERMINATE_GRACE_PERIOD, 5000); + int sigkillDelayOnStop = IntentUtils.getIntegerExtraIfSet(intent, TERMUX_SERVICE.EXTRA_SIGKILL_DELAY_ON_STOP, 5000); AppShell appShell = getTermuxTaskForShellName(shellName); while (appShell != null) { - appShell.terminateIfExecuting(getApplicationContext(), gracePeriod, true); + appShell.terminateIfExecuting(getApplicationContext(), sigkillDelayOnStop, true); appShell = getTermuxTaskForShellName(shellName); } } diff --git a/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java b/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java index 3592873e32..a37c7827d2 100644 --- a/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java +++ b/termux-shared/src/main/java/com/termux/shared/shell/command/runner/app/AppShell.java @@ -248,16 +248,16 @@ private void executeInner(@NonNull final Context context) throws IllegalThreadSt /** * Terminate this {@link AppShell} by sending a {@link OsConstants#SIGTERM} to its {@link #mProcess} - * if it is still executing. After {@code gracePeriodMsec} milliseconds {@link OsConstants#SIGTERM} is + * if it is still executing. After {@code sigkillDelayOnStop} milliseconds {@link OsConstants#SIGTERM} is * signalled. * * @param context The {@link Context} for operations. - * @param gracePeriodMsec The delay after which a SIGKILL is send. + * @param sigkillDelayOnStop The delay after which a SIGKILL is send. * @param processResult If set to {@code true}, then the {@link #processAppShellResult(AppShell, ExecutionCommand)} * will be called to process the failure. */ - public void terminateIfExecuting(@NonNull final Context context, long gracePeriodMsec, boolean processResult) { - if (gracePeriodMsec == 0) { + public void terminateIfExecuting(@NonNull final Context context, long sigkillDelayOnStop, boolean processResult) { + if (sigkillDelayOnStop == 0) { killIfExecuting(context, processResult); return; } @@ -274,7 +274,7 @@ public void terminateIfExecuting(@NonNull final Context context, long gracePerio term(); } - (new Handler()).postDelayed(() -> killIfExecuting(context, processResult), gracePeriodMsec); + (new Handler()).postDelayed(() -> killIfExecuting(context, processResult), sigkillDelayOnStop); } /** diff --git a/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java b/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java index df3c311fc7..8d4a5ceccb 100644 --- a/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java +++ b/termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java @@ -1049,9 +1049,9 @@ public static final class TERMUX_SERVICE { * be created in {@link #EXTRA_RESULT_DIRECTORY} if {@link #EXTRA_RESULT_SINGLE_FILE} is * {@code false} for the TERMUX_SERVICE.ACTION_SERVICE_EXECUTE intent */ public static final String EXTRA_RESULT_FILES_SUFFIX = TERMUX_PACKAGE_NAME + ".execute.result_files_suffix"; // Default: "com.termux.execute.result_files_suffix" - /** Intent {@code long} extra for graceperiod between SIGTERM and SIGKILL + /** Intent {@code long} extra for the delay between SIGTERM and SIGKILL * for the TERMUX_SERVICE.ACTION_SERVICE_STOP intent */ - public static final String EXTRA_TERMINATE_GRACE_PERIOD = TERMUX_PACKAGE_NAME + ".execute.stop_delay"; // Default: "com.termux.execute.stop_delay" + public static final String EXTRA_SIGKILL_DELAY_ON_STOP = TERMUX_PACKAGE_NAME + ".execute.sigkill_delay_on_stop"; // Default: "com.termux.execute.sigkill_delay_on_stop"