From 77bd2dfb9facbfb9bd08c7be964280d4ce6b78b7 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 14 Feb 2018 22:29:11 -0700 Subject: [PATCH] Better handling of NTP-based clocks. Now that we have a nice Clock abstraction, we can use it to represent a clock backed by an NTP fix. (This makes testing logic much easier to write.) We now rely completely on NetworkTimeUpdateService to keep our NTP fix up to date, instead of trying to refresh in the middle of critical paths which could trigger random ANRs. Add internal FallbackClock to make it easier to handle missing NTP fixes. Add internal SimpleClock to let implementers focus on single millis() method. Test: bit FrameworksNetTests:com.android.server.net.NetworkStatsServiceTest Test: bit FrameworksServicesTests:com.android.server.NetworkPolicyManagerServiceTest Bug: 69714690, 72320957 Change-Id: Ic32cdcbe093d08b73b0e4b23d6910b23ea8e1968 Exempt-From-Owner-Approval: approved in previous PS --- .../server/net/NetworkStatsService.java | 51 ++++++++----------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 76c4db1340..d1aa21276b 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -68,6 +68,7 @@ import static com.android.server.NetworkManagementService.LIMIT_GLOBAL_ALERT; import static com.android.server.NetworkManagementSocketTagger.resetKernelUidStats; import static com.android.server.NetworkManagementSocketTagger.setKernelCounterSet; +import android.annotation.NonNull; import android.app.AlarmManager; import android.app.PendingIntent; import android.app.usage.NetworkStatsManager; @@ -97,6 +98,7 @@ import android.net.TrafficStats; import android.os.Binder; import android.os.DropBoxManager; import android.os.Environment; +import android.os.BestClock; import android.os.Handler; import android.os.HandlerThread; import android.os.IBinder; @@ -120,10 +122,8 @@ import android.util.ArraySet; import android.util.EventLog; import android.util.Log; import android.util.MathUtils; -import android.util.NtpTrustedTime; import android.util.Slog; import android.util.SparseIntArray; -import android.util.TrustedTime; import android.util.proto.ProtoOutputStream; import com.android.internal.annotations.GuardedBy; @@ -140,6 +140,8 @@ import java.io.File; import java.io.FileDescriptor; import java.io.IOException; import java.io.PrintWriter; +import java.time.Clock; +import java.time.ZoneOffset; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -167,7 +169,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private final Context mContext; private final INetworkManagementService mNetworkManager; private final AlarmManager mAlarmManager; - private final TrustedTime mTime; + private final Clock mClock; private final TelephonyManager mTeleManager; private final NetworkStatsSettings mSettings; private final NetworkStatsObservers mStatsObservers; @@ -202,7 +204,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { */ public interface NetworkStatsSettings { public long getPollInterval(); - public long getTimeCacheMaxAge(); public boolean getSampleEnabled(); public boolean getAugmentEnabled(); @@ -281,16 +282,21 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private long mPersistThreshold = 2 * MB_IN_BYTES; private long mGlobalAlertBytes; - private static File getDefaultSystemDir() { + private static @NonNull File getDefaultSystemDir() { return new File(Environment.getDataDirectory(), "system"); } - private static File getDefaultBaseDir() { + private static @NonNull File getDefaultBaseDir() { File baseDir = new File(getDefaultSystemDir(), "netstats"); baseDir.mkdirs(); return baseDir; } + private static @NonNull Clock getDefaultClock() { + return new BestClock(ZoneOffset.UTC, SystemClock.currentNetworkTimeClock(), + Clock.systemUTC()); + } + public static NetworkStatsService create(Context context, INetworkManagementService networkManager) { AlarmManager alarmManager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE); @@ -299,7 +305,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG); NetworkStatsService service = new NetworkStatsService(context, networkManager, alarmManager, - wakeLock, NtpTrustedTime.getInstance(context), TelephonyManager.getDefault(), + wakeLock, getDefaultClock(), TelephonyManager.getDefault(), new DefaultNetworkStatsSettings(context), new NetworkStatsObservers(), getDefaultSystemDir(), getDefaultBaseDir()); @@ -313,13 +319,13 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @VisibleForTesting NetworkStatsService(Context context, INetworkManagementService networkManager, - AlarmManager alarmManager, PowerManager.WakeLock wakeLock, TrustedTime time, + AlarmManager alarmManager, PowerManager.WakeLock wakeLock, Clock clock, TelephonyManager teleManager, NetworkStatsSettings settings, NetworkStatsObservers statsObservers, File systemDir, File baseDir) { mContext = checkNotNull(context, "missing Context"); mNetworkManager = checkNotNull(networkManager, "missing INetworkManagementService"); mAlarmManager = checkNotNull(alarmManager, "missing AlarmManager"); - mTime = checkNotNull(time, "missing TrustedTime"); + mClock = checkNotNull(clock, "missing Clock"); mSettings = checkNotNull(settings, "missing NetworkStatsSettings"); mTeleManager = checkNotNull(teleManager, "missing TelephonyManager"); mWakeLock = checkNotNull(wakeLock, "missing WakeLock"); @@ -413,8 +419,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mContext.unregisterReceiver(mUserReceiver); mContext.unregisterReceiver(mShutdownReceiver); - final long currentTime = mTime.hasCache() ? mTime.currentTimeMillis() - : System.currentTimeMillis(); + final long currentTime = mClock.millis(); // persist any pending stats mDevRecorder.forcePersistLocked(currentTime); @@ -831,8 +836,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } // update and persist if beyond new thresholds - final long currentTime = mTime.hasCache() ? mTime.currentTimeMillis() - : System.currentTimeMillis(); + final long currentTime = mClock.millis(); synchronized (mStatsLock) { if (!mSystemReady) return; @@ -1170,8 +1174,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { */ @GuardedBy("mStatsLock") private void bootstrapStatsLocked() { - final long currentTime = mTime.hasCache() ? mTime.currentTimeMillis() - : System.currentTimeMillis(); + final long currentTime = mClock.millis(); try { recordSnapshotLocked(currentTime); @@ -1183,11 +1186,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } private void performPoll(int flags) { - // try refreshing time source when stale - if (mTime.getCacheAge() > mSettings.getTimeCacheMaxAge()) { - mTime.forceRefresh(); - } - synchronized (mStatsLock) { mWakeLock.acquire(); @@ -1215,8 +1213,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { final boolean persistForce = (flags & FLAG_PERSIST_FORCE) != 0; // TODO: consider marking "untrusted" times in historical stats - final long currentTime = mTime.hasCache() ? mTime.currentTimeMillis() - : System.currentTimeMillis(); + final long currentTime = mClock.millis(); try { recordSnapshotLocked(currentTime); @@ -1268,7 +1265,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @GuardedBy("mStatsLock") private void performSampleLocked() { // TODO: migrate trustedtime fixes to separate binary log events - final long trustedTime = mTime.hasCache() ? mTime.currentTimeMillis() : -1; + final long currentTime = mClock.millis(); NetworkTemplate template; NetworkStats.Entry devTotal; @@ -1285,7 +1282,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { devTotal.rxBytes, devTotal.rxPackets, devTotal.txBytes, devTotal.txPackets, xtTotal.rxBytes, xtTotal.rxPackets, xtTotal.txBytes, xtTotal.txPackets, uidTotal.rxBytes, uidTotal.rxPackets, uidTotal.txBytes, uidTotal.txPackets, - trustedTime); + currentTime); // collect wifi sample template = buildTemplateWifiWildcard(); @@ -1297,7 +1294,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { devTotal.rxBytes, devTotal.rxPackets, devTotal.txBytes, devTotal.txPackets, xtTotal.rxBytes, xtTotal.rxPackets, xtTotal.txBytes, xtTotal.txPackets, uidTotal.rxBytes, uidTotal.rxPackets, uidTotal.txBytes, uidTotal.txPackets, - trustedTime); + currentTime); } /** @@ -1621,10 +1618,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { return getGlobalLong(NETSTATS_POLL_INTERVAL, 30 * MINUTE_IN_MILLIS); } @Override - public long getTimeCacheMaxAge() { - return getGlobalLong(NETSTATS_TIME_CACHE_MAX_AGE, DAY_IN_MILLIS); - } - @Override public long getGlobalAlertBytes(long def) { return getGlobalLong(NETSTATS_GLOBAL_ALERT_BYTES, def); }