Skip to content

Commit

Permalink
Bug 1910449 - Use daemon-thread with MPS timer instead of user-thread
Browse files Browse the repository at this point in the history
Use a daemon-thread instead of the default user-thread in the `java.util.Timer` used to schedule pings within the MetricsPingScheduler. User-threads can prevent the application from terminating gracefully while daemon-threads do not.
  • Loading branch information
travis79 committed Aug 20, 2024
1 parent c61498d commit 4af2424
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package mozilla.telemetry.glean

import androidx.annotation.VisibleForTesting
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.DelicateCoroutinesApi
import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.launch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ internal class OnGleanEventsImpl(val glean: GleanInternalAPI) : OnGleanEvents {
}

override fun cancelUploads() {
// Cancel any pending workers here so that we don't accidentally upload or
// collect data after the upload has been disabled.
// Cancel any pending metrics ping scheduler tasks
glean.metricsPingScheduler?.cancel()
// Cancel any pending workers here so that we don't accidentally upload
// data after the upload has been disabled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,17 @@ internal class MetricsPingScheduler(
val millisUntilNextDueTime = getMillisecondsUntilDueTime(sendTheNextCalendarDay, now)
Log.d(LOG_TAG, "Scheduling the 'metrics' ping in ${millisUntilNextDueTime}ms")

// Cancel any existing scheduled work. Does not actually ancel a
// currently-running task.
// Cancel the MPS scheduler, discarding any currently scheduled tasks. This does not
// interfere with a currently executing task (if one exists).
cancel()

timer = Timer("glean.MetricsPingScheduler")
// After cancelling the MPS timer, it will no longer be able to schedule new tasks for
// execution, so we will need to create a new one in order to schedule more work. This is
// done using`isDaemon: true`. We configure the timer to use a daemon-thread because it will
// not prevent the application from terminating gracefully, unlike the default user-thread.
// See: (https://developer.android.com/reference/java/util/Timer#Timer(boolean))
timer = Timer("glean.MetricsPingScheduler", true)

timer?.schedule(MetricsPingTimer(this, reason), millisUntilNextDueTime)
}

Expand Down Expand Up @@ -311,8 +317,13 @@ internal class MetricsPingScheduler(
// Update the collection date: we don't really care if we have data or not, let's
// always update the sent date.
updateSentDate(getISOTimeString(now, truncateTo = TimeUnit.DAY))
// Reschedule the collection.
schedulePingCollection(now, sendTheNextCalendarDay = true, reason = Pings.metricsReasonCodes.reschedule)

// Schedule the next metrics ping collection
schedulePingCollection(
now,
sendTheNextCalendarDay = true,
reason = Pings.metricsReasonCodes.reschedule,
)
}

/**
Expand Down Expand Up @@ -340,6 +351,10 @@ internal class MetricsPingScheduler(
* Function to cancel any pending metrics ping timers
*/
fun cancel() {
// Terminate the MPS timer object, discarding any currently scheduled tasks. Does not
// interfere with a currently executing task (if it exists). Once a timer has been
// terminated, its execution thread terminates gracefully, and no more tasks may be
// scheduled on it.
timer?.cancel()
timer = null
}
Expand Down

0 comments on commit 4af2424

Please sign in to comment.