Bug 1910449 - Use daemon-threads with the MPS timer instead of the default user-threads

[travis79]

Changing the MPS java.util.Timer to use daemon-threads instead of the default user-threads because user-threads can potentially prevent the application from terminating properly since the OS requires all user-threads to be joined before the app can terminate. Daemon-threads are terminated by the OS once all user-threads have completed and are not awaited before the application can terminate.

[travis79]

This requires a bit of communication with product and DS folks before landing and release to ensure no one is surprised by any changes to the data, expected or unexpected.

[badboy]

As called out in the bug what about this trivial patch:

diff --git glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/MetricsPingScheduler.kt glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/MetricsPingScheduler.kt
index 6387ceff5..0ed70bed8 100644
--- glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/MetricsPingScheduler.kt
+++ glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/MetricsPingScheduler.kt
@@ -107,7 +107,9 @@ internal class MetricsPingScheduler(
         // currently-running task.
         cancel()
 
-        timer = Timer("glean.MetricsPingScheduler")
+        if (timer == null) {
+            timer = Timer("glean.MetricsPingScheduler", true)
+        }
         timer?.schedule(MetricsPingTimer(this, reason), millisUntilNextDueTime)
     }

Not sure how you tested this change, but if you have a strategy for that maybe with the above daemon thread we can test again?

[travis79]

Not sure how you tested this change, but if you have a strategy for that maybe with the above daemon thread we can test again?

Well... it involved an actual device and about a week of letting it sit for about 2 days at a time waiting for things to happen naturally 😅! It was very manual and time consuming due to the 1/day scheduling, but it seemed to behave as I intended. I'm not even sure all devices and Android versions behave the same with regards to apps running in the background, making this even more "fun" to tinker with.

I'm still set up to test this so I'll try the daemon thread approach as an option to this, since it seems like it would be an even cleaner approach than relying on the lifecycle detection & cancellation.

[travis79]

timer = Timer("glean.MetricsPingScheduler")
        if (timer == null) {
            timer = Timer("glean.MetricsPingScheduler", true)
        }

Turns out the timer is always going to be null here because of the call to cancel() right before (which nulls out the timer. So, I don't think this is necessary to add the check before creating a new Timer anyway.