Skip to content

Commit

Permalink
Bug 1910449 - Add explicit call to cancel for MPS (java.util.Timer) i…
Browse files Browse the repository at this point in the history
…n Glean Android bindings

This adds an explicit call to `Timer.cancel()` for the Metrics Ping Scheduler's  timer when the application enters the background (`handleClientInactive`) in order to prevent the Timer thread from blocking the application from termination (see `java.util.Timer` docs here: https://developer.android.com/reference/java/util/Timer)

This also adds a check to ensure the application is running and in the foreground when the `Timer` fires before rescheduling the next metrics ping `Timer`
  • Loading branch information
travis79 committed Aug 19, 2024
1 parent c7b7ce3 commit 7e74f7f
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 5 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 @@ -320,7 +320,7 @@ open class GleanInternalAPI internal constructor() {
branch: String,
extra: Map<String, String>? = null,
) {
var map = extra ?: mapOf()
val map = extra ?: mapOf()
gleanSetExperimentActive(experimentId, branch, map)
}

Expand Down Expand Up @@ -448,6 +448,12 @@ open class GleanInternalAPI internal constructor() {
// Persist data on backgrounding the app
persistPingLifetimeData()

// The MetricsPingScheduler makes use of java.util.Timer, which has the
// potential to keep the app from terminating. So, we cancel the MPS when
// the app transitions to background to try and prevent this.
// See https://developer.android.com/reference/java/util/Timer
metricsPingScheduler?.cancel()

gleanHandleClientInactive()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

package mozilla.telemetry.glean.scheduler

import android.app.ActivityManager
import android.app.ActivityManager.RunningAppProcessInfo
import android.content.Context
import android.content.SharedPreferences
import android.text.format.DateUtils
Expand Down Expand Up @@ -103,7 +105,7 @@ 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
// Cancel any existing scheduled work. Does not actually cancel a
// currently-running task.
cancel()

Expand Down Expand Up @@ -311,8 +313,20 @@ 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)
// The MetricsPingScheduler makes use of java.util.Timer, which has the potential to
// keep the app from terminating. We only reschedule if the application is in the
// foreground, otherwise we cancel the MPS to try and prevent it from blocking
// application termination.
// See https://developer.android.com/reference/java/util/Timer
if (isAppOnForeground()) {
schedulePingCollection(
now,
sendTheNextCalendarDay = true,
reason = Pings.metricsReasonCodes.reschedule,
)
} else {
cancel()
}
}

/**
Expand Down Expand Up @@ -363,6 +377,23 @@ internal class MetricsPingScheduler(
*/
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun getCalendarInstance(): Calendar = Calendar.getInstance()

/**
* Utility function used in determining whether or not to reschedule the MPS
*/
private fun isAppOnForeground(): Boolean {
val activityManager = applicationContext.getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager
val appProcesses = activityManager.runningAppProcesses ?: return false
val packageName = applicationContext.packageName
for (appProcess in appProcesses) {
if (appProcess.importance ==
RunningAppProcessInfo.IMPORTANCE_FOREGROUND && appProcess.processName == packageName
) {
return true
}
}
return false
}
}

/**
Expand Down

0 comments on commit 7e74f7f

Please sign in to comment.