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

Fix RunCommandService crash and foreground issue #1764

Conversation

agnostic-apollo
Copy link
Member

Since startForeground() function is not being called, occasionally termux is killed by android with the following exception

I/ActivityTaskManager: START u0 {flg=0x10000000 cmp=com.termux/.app.TermuxActivity} from uid 10138
W/ActivityTaskManager: Background activity start [callingPackage: com.termux; callingUid: 10138; isCallingUidForeground: false; isCallingUidPersistentSystemProcess: false; realCallingUid: 10138; isRealCallingUidForeground: false; isRealCallingUidPersistentSystemProcess: false; originatingPendingIntent: null; isBgStartWhitelisted: false; intent: Intent { flg=0x10000000 cmp=com.termux/.app.TermuxActivity }; callerApp: ProcessRecord{75609ea 14280:com.termux/u0a138}]
D/AndroidRuntime: Shutting down VM
E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.termux, PID: 14280
    android.app.RemoteServiceException: Context.startForegroundService() did not then call Service.startForeground(): ServiceRecord{f94f200 u0 com.termux/.app.RunCommandService}
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1945)
        at android.os.Handler.dispatchMessage(Handler.java:107)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7356)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)
I/ActivityManager: Showing crash dialog for package com.termux u0
W/ActivityManager: Bringing down service while still waiting for start foreground: ServiceRecord{d0f1bbf u0 com.termux/.app.RunCommandService}
I/ActivityManager: Crashing app skipping ANR: ProcessRecord{75609ea 14280:com.termux/u0a138} Context.startForegroundService() did not then call Service.startForeground(): ServiceRecord{d0f1bbf u0 com.termux/.app.RunCommandService}

I have added calls to startForeground() in both onCreate() and onStartCommand() and have tested the in android 10 emulator and the fix seems to be working with no crashes yet. I even tested under low free RAM conditions in order to slow down the phone but termux still didn't crash. Of course, it could potentially still crash because the function may not be called in under 5s in all circumstances since its a framework issue. More details here. Disabling battery optimization is likely the better way but this also works and is the recommended way.

Adding android.permission.SYSTEM_ALERT_WINDOW permission to fix the foreground issue seems to be the reasonable way to go since all the other exceptions to the rule do not really apply to termux, and without it, it greatly reduces usability of both the RUN_COMMAND intents and Termux:Tasker plugin. If a user has already set a certain command to be run in a foreground session from a background task like from tasker, it should run in foreground without user intervention. The hack that currently works for running termux commands from tasker is to do a sleep 1; am start --user 0 -n com.termux/com.termux.app.TermuxActivity after running the Termux:Tasker plugin or TermuxCommand action, since tasker already has requested android.permission.SYSTEM_ALERT_WINDOW permission and also has an accessibility service, so it can actually start termux activity from its own background execution service, but termux itself can't from its own service. I haven't currently added a way for the user to request the permission from within termux, let me know if that is even required.

Let me know if there are any other issues. Thanks

@Grimler91
Copy link
Member

Hey, thanks! Could you maybe split the commit into two or three parts to keep the git history easier to follow? (For example fixing the crash in one, improving RUN_COMMAND functionality in one and adding the PREFIX/HOME expanding in one)

Will test this out on my devices!

@agnostic-apollo
Copy link
Member Author

Sure. Out at the moment, will do it when I get back.

…startForeground()" function is not being called before running "startForegroundService()" in RunCommandService.
…lly in android >= 10 unless user manually clicks termux notification for "RUN_COMMAND" intents and "Termux:Tasker" plugin actions that have background mode "false" because of new restrictions of starting activities from background. This is done by adding "android.permission.SYSTEM_ALERT_WINDOW" permission in AndroidManifest.xml so that the user may optionally grant "Draw Over Apps" permission to termux to fix the issue.
@agnostic-apollo agnostic-apollo force-pushed the termux-run-command-crash-and-foreground-patch branch from 1f6ae46 to 7063a3a Compare September 18, 2020 17:31
@agnostic-apollo
Copy link
Member Author

agnostic-apollo commented Sep 18, 2020

Hey, these commits now should be the same as the original single commit. Let me know if its working fine on your devices. I don't have a physical android >= 8 device.

And forgot to mention, to produce the crash on v0.99 of termux, exit all termux sessions, remove termux from recents, wait a few minutes so that likely the timeout for the allowlist passes, then use Tasker TermuxCommand function in Tasker Function action to run a command, and of course cross your fingers. ;) For arguments to work, use Tasker v5.9.3 sub release. am command should work too, but in that case if I remember correctly errors were shown in stderr and logcat had entries like Background start not allowed: service Intent { act=com.termux.RUN_COMMAND cmp=com.termux/.app.RunCommandService (has extras) } to com.termux/.app.RunCommandService from pid=9616 uid=10134 pkg=com.android.shell startFg?=false.

@Grimler91
Copy link
Member

@agnostic-apollo I have not been able to reproduce the crash on my android 10 (LineageOS) device, tasker reports another error:

19.08.55/E FIRE PLUGIN: Termux / com.twofortyfouram.locale.intent.action.FIRE_SETTING: 6 bundle keys
19.08.55/E Termux: plugin comp: com.termux.tasker/com.termux.tasker.FireReceiver
19.08.55/E add wait type Plugin1 time 10
19.08.55/E add wait type Plugin1 done
19.08.55/E add wait task
19.09.05/E Error: null

This PR does not change anything on that device, but merging since it fixes the crash on your device.

@Grimler91 Grimler91 merged commit eb7fda2 into termux:master Sep 27, 2020
@agnostic-apollo
Copy link
Member Author

Well, u/DutchOfBurdock was the one who initially reported the issue on his device which I confirmed on android 10 avd, so the issue does exist for others. I personally use LG G5, Android 7.0 which obv doesn't have the issue.

hmm, interesting that you can't reproduce it. LineageOS doesn't seem to have looser restrictions for background stuff according to this report. I have already mentioned this, but just confirming that have you disabled termux battery optimizations, if you disable those, then you are likely not going to get a crash.

The entries you are getting by tasker are from its ExecuteService which has the tag E, they are not errors, just entries that the service is adding a wait timeout for 10s for the plugin action.

And thanks for the merge.

@ghost
Copy link

ghost commented Sep 27, 2020

Part of v0.100 release.

@agnostic-apollo
Copy link
Member Author

Thanks. Congrats on the 0.100th though. And thank you for everything you guys have done.

@Grimler91
Copy link
Member

confirming that have you disabled termux battery optimizations, if you disable those, then you are likely not going to get a crash

Ah, yes, I was running without battery optimization, so that is probably the reason.. Should have read the instructions more carefully..

The entries you are getting by tasker are from its ExecuteService which has the tag E, they are not errors, just entries that the service is adding a wait timeout for 10s for the plugin action.

Alright, I get a timeout then without my test script being executed, thanks for clarifying that!

@agnostic-apollo
Copy link
Member Author

Glad you managed to get a crash ;)

You are welcome.

@agnostic-apollo agnostic-apollo deleted the termux-run-command-crash-and-foreground-patch branch February 14, 2021 11:39
AdamMickiewich pushed a commit to VolyaTeam/dzida-app that referenced this pull request Aug 8, 2022
…d-crash-and-foreground-patch

Fix RunCommandService crash and foreground issue
shrihankp pushed a commit to reisxd/termux-app that referenced this pull request Oct 20, 2022
…d-crash-and-foreground-patch

Fix RunCommandService crash and foreground issue
@termux termux deleted a comment from Davidrekrut Nov 9, 2023
@TomJo2000
Copy link
Member

today is the best day

Because a random bug fix from 3 years ago?

@Davidrekrut
Copy link

Мне нравится

@Davidrekrut
Copy link

Великолепное чудо

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants