Closed chenxiaolong closed 1 month ago
tmo1
commented
1 month ago Wow, thanks so much for all this! So IIUC, the main problem here was that the call to schedule the next export job was keeping the foreground service running - indefinitely?! Why? It had occurred to me that that could somehow be the problem, but I don't get it: I would think that after the call to schedule the job returns, the foreground service should end.
chenxiaolong
commented
1 month ago No problem!
I would think that after the call to schedule the job returns, the foreground service should end.
I did some further investigation into this and I think I have a better idea of what's going on now.
With or without the workaround, the service actually does exit. I confirmed this by running:
adb shell dumpsys activity services sms_ieduring and after the scheduled export. After the export completes, the search returns nothing. It seems like the only negative side effect without the workaround is that the notification not being dismissed, not any additional battery drain or anything like that.
So then I enabled the androidx work library's debug logging with: https://developer.android.com/develop/background-work/background-tasks/testing/persistent/debug. The step for what to add to AndroidManifest.xml is actually wrong and should be:
<provider
android:name="androidx.startup.InitializationProvider"
android:exported="false"
android:authorities="${applicationId}.androidx-startup">
<meta-data
android:name="androidx.work.WorkManagerInitializer"
android:value="androidx.startup"
tools:node="remove" />
</provider>With debug debug logging in place, this is what happens for the scheduled exports with and without the workaround:
The thing that caught my eye is that without the workaround, the library is treating the job as being cancelled (Work interrupted for Work) instead of completing successfully (Worker result SUCCESS for Work). Somehow this prevents the library for executing the logic for clearing the notification (Removing Notification (id: 0<...>).
I guess it's just undocumented behavior that scheduling a new job within a running job causes the running job to be cancelled and that cancelled jobs don't dismiss the foreground notification.
chenxiaolong
commented
1 month ago (I will update the commit description to reflect what I've posted here ^^)
EDIT: Done!
chenxiaolong
commented
1 month ago Oh wait! The job is getting cancelled because updateExportWork() is explicitly cancelling it. I completely missed that.
I've updated the branch with a new approach. 6d988c7f7e6ea7fd53baa6615c474b9a0c854054 should fix this properly (simpler and without any ugly workarounds).
tmo1
commented
1 month ago So ultimately it was at least partially my fault, for not remebering that we were cancelling the old job when rescheduling the new one (and not realizng the ramifications thereof) and the fix was simple: don't cancel when rescheduling upon job completion. Thanks so much for figuring this out!
The truth is that I really don't much like the current paradigm of continually rescheduling the next job upon completion of the previous one. It feels hackish, and if something goes wrong and a job crashes, I suppose that no further ones will ever get scheduled without manual intervention. But as you note, there doesn't seem to be a good, clean alternative in Android.
chenxiaolong
commented
1 month ago Yep, I think the best we can do for the crash scenario is to just move the scheduling to the beginning of doWork() (or try/catch the whole thing).
tmo1
commented
1 month ago Yep, I think the best we can do for the crash scenario is to just move the scheduling to the beginning of doWork() (or try/catch the whole thing).
I think I'll try the former when I get a chance. I just can't believe that Android is making me reimplement cron, badly :|
Thanks again for all the help.
Please see the individual commits for more details.
Changes:
Fixes: #34 Fixes: #179 Probably improves: #183