Add ACTION_SERVICE_STOP intent to only stop a single AppShell #3821
Conversation
|
|
||
| Uri executableUri = intent.getData(); | ||
| String executable = UriUtils.getUriFilePathWithFragment(executableUri); | ||
| String shellName = ShellUtils.getExecutableBasename(executable); |
There was a problem hiding this comment.
There would be conflicts if same executable was run from someplace else, and that function only returns first one. You should generate at least a six character alphanumeric [a-zA-Z0-9] random string and store that in cron tab in addition to the id and when sending execute or stop intent, pass that as TERMUX_SERVICE.EXTRA_SHELL_NAME, so that the executable for the job is killed, and not any other.
There was a problem hiding this comment.
Good point.
Is there a character limit on the shell name?
Or the other way around: Could I just use a UUID?
There was a problem hiding this comment.
There is no limit currently, but there should be, ideally 255, equal to NAME_NAME.
UUID would be too long and harder to see in app shells UI list and logs, just use a six character alphanumeric [a-zA-Z0-9] string, like mktemp template uses for unique files. Following should work, make sure <script_name>_<public_id>_<private_id> doesn't already exist in cron tab, otherwise regenerate private_id. The public_id here is the incrementing number you are currently using.
char[] allowedCharsArray = ("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789").toCharArray();
char[] private_id = new char[6];
Random random = new SecureRandom();
for (int i = 0; i < private_id.length; i++) {
private_id[i] = allowedCharsArray[random.nextInt(allowedCharsArray.length)];
}There was a problem hiding this comment.
I see. Thanks for the example.
I changed the method to only work if the shell name is explicitly set.
There was a problem hiding this comment.
Welcome. Makes sense. I think it would be better to call getTermuxTaskForShellName() in a while loop until it starts returning null, so that all shells with same name get killed, since currently only first one would get killed, which may not be the one caller intended to kill. To prevent duplicates, callers should start shells with a unique shell name, like the cron API would do.
There was a problem hiding this comment.
Good point - I had not considered multiple shells with the same name.
termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java
Outdated
Show resolved
Hide resolved
| @@ -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"; // Default: "com.termux.execute.stop.delay" | |||
There was a problem hiding this comment.
This should be stop_delay
There was a problem hiding this comment.
The extra name was also different. Both should be same with respective case, naming them sigkill_delay_on_stop makes it more clear on function of extra.
There was a problem hiding this comment.
Will have to rename gracePeriod and gracePeriodMsec as well
* fix stop_delay constant * terminate all appshells with matching name
This adds the possibility to basically stop a running AppShell that was created by sending a ACTION_SERVICE_EXECUTE intent.
This is used by my ongoing implementation of a cron-like service into TermuxAPI (PR: termux/termux-api#657)