diff --git a/service-t/src/com/android/server/net/NetworkStatsRecorder.java b/service-t/src/com/android/server/net/NetworkStatsRecorder.java index d99e164cd6..7c801d7a94 100644 --- a/service-t/src/com/android/server/net/NetworkStatsRecorder.java +++ b/service-t/src/com/android/server/net/NetworkStatsRecorder.java @@ -78,6 +78,7 @@ public class NetworkStatsRecorder { private final long mBucketDuration; private final boolean mOnlyTags; + private final boolean mWipeOnError; private long mPersistThresholdBytes = 2 * MB_IN_BYTES; private NetworkStats mLastSnapshot; @@ -102,6 +103,7 @@ public class NetworkStatsRecorder { // slack to avoid overflow mBucketDuration = YEAR_IN_MILLIS; mOnlyTags = false; + mWipeOnError = true; mPending = null; mSinceBoot = new NetworkStatsCollection(mBucketDuration); @@ -113,7 +115,8 @@ public class NetworkStatsRecorder { * Persisted recorder. */ public NetworkStatsRecorder(FileRotator rotator, NonMonotonicObserver observer, - DropBoxManager dropBox, String cookie, long bucketDuration, boolean onlyTags) { + DropBoxManager dropBox, String cookie, long bucketDuration, boolean onlyTags, + boolean wipeOnError) { mRotator = Objects.requireNonNull(rotator, "missing FileRotator"); mObserver = Objects.requireNonNull(observer, "missing NonMonotonicObserver"); mDropBox = Objects.requireNonNull(dropBox, "missing DropBoxManager"); @@ -121,6 +124,7 @@ public class NetworkStatsRecorder { mBucketDuration = bucketDuration; mOnlyTags = onlyTags; + mWipeOnError = wipeOnError; mPending = new NetworkStatsCollection(bucketDuration); mSinceBoot = new NetworkStatsCollection(bucketDuration); @@ -552,7 +556,9 @@ public class NetworkStatsRecorder { } mDropBox.addData(TAG_NETSTATS_DUMP, os.toByteArray(), 0); } - - mRotator.deleteAll(); + // Delete all files if this recorder is set wipe on error. + if (mWipeOnError) { + mRotator.deleteAll(); + } } } diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 2bf3ab9772..02f268b533 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -800,11 +800,14 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mSystemReady = true; // create data recorders along with historical rotators - mDevRecorder = buildRecorder(PREFIX_DEV, mSettings.getDevConfig(), false, mStatsDir); - mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, mStatsDir); - mUidRecorder = buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, mStatsDir); + mDevRecorder = buildRecorder(PREFIX_DEV, mSettings.getDevConfig(), false, mStatsDir, + true /* wipeOnError */); + mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, mStatsDir, + true /* wipeOnError */); + mUidRecorder = buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, mStatsDir, + true /* wipeOnError */); mUidTagRecorder = buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, - mStatsDir); + mStatsDir, true /* wipeOnError */); updatePersistThresholdsLocked(); @@ -869,12 +872,13 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private NetworkStatsRecorder buildRecorder( String prefix, NetworkStatsSettings.Config config, boolean includeTags, - File baseDir) { + File baseDir, boolean wipeOnError) { final DropBoxManager dropBox = (DropBoxManager) mContext.getSystemService( Context.DROPBOX_SERVICE); return new NetworkStatsRecorder(new FileRotator( baseDir, prefix, config.rotateAgeMillis, config.deleteAgeMillis), - mNonMonotonicObserver, dropBox, prefix, config.bucketDuration, includeTags); + mNonMonotonicObserver, dropBox, prefix, config.bucketDuration, includeTags, + wipeOnError); } @GuardedBy("mStatsLock") @@ -971,12 +975,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub { final NetworkStatsRecorder[] legacyRecorders; if (runComparison) { final File legacyBaseDir = mDeps.getLegacyStatsDir(); + // Set wipeOnError flag false so the recorder won't damage persistent data if reads + // failed and calling deleteAll. legacyRecorders = new NetworkStatsRecorder[]{ - buildRecorder(PREFIX_DEV, mSettings.getDevConfig(), false, legacyBaseDir), - buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, legacyBaseDir), - buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, legacyBaseDir), - buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, legacyBaseDir) - }; + buildRecorder(PREFIX_DEV, mSettings.getDevConfig(), false, legacyBaseDir, + false /* wipeOnError */), + buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, legacyBaseDir, + false /* wipeOnError */), + buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, legacyBaseDir, + false /* wipeOnError */), + buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, legacyBaseDir, + false /* wipeOnError */)}; } else { legacyRecorders = null; } diff --git a/tests/unit/java/android/net/NetworkStatsRecorderTest.java b/tests/unit/java/android/net/NetworkStatsRecorderTest.java new file mode 100644 index 0000000000..fad11a3ff7 --- /dev/null +++ b/tests/unit/java/android/net/NetworkStatsRecorderTest.java @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + *i + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.net; + +import static android.text.format.DateUtils.HOUR_IN_MILLIS; + +import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; + +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyLong; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import android.net.NetworkStats; +import android.os.DropBoxManager; + +import androidx.test.filters.SmallTest; + +import com.android.internal.util.FileRotator; +import com.android.testutils.DevSdkIgnoreRule; +import com.android.testutils.DevSdkIgnoreRunner; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.io.IOException; + +@RunWith(DevSdkIgnoreRunner.class) +@SmallTest +@DevSdkIgnoreRule.IgnoreUpTo(SC_V2) +public final class NetworkStatsRecorderTest { + private static final String TAG = NetworkStatsRecorderTest.class.getSimpleName(); + + private static final String TEST_PREFIX = "test"; + + @Mock private DropBoxManager mDropBox; + @Mock private NetworkStats.NonMonotonicObserver mObserver; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + } + + private NetworkStatsRecorder buildRecorder(FileRotator rotator, boolean wipeOnError) { + return new NetworkStatsRecorder(rotator, mObserver, mDropBox, TEST_PREFIX, + HOUR_IN_MILLIS, false /* includeTags */, wipeOnError); + } + + @Test + public void testWipeOnError() throws Exception { + final FileRotator rotator = mock(FileRotator.class); + final NetworkStatsRecorder wipeOnErrorRecorder = buildRecorder(rotator, true); + + // Assuming that the rotator gets an exception happened when read data. + doThrow(new IOException()).when(rotator).readMatching(any(), anyLong(), anyLong()); + wipeOnErrorRecorder.getOrLoadPartialLocked(Long.MIN_VALUE, Long.MAX_VALUE); + // Verify that the files will be deleted. + verify(rotator, times(1)).deleteAll(); + reset(rotator); + + final NetworkStatsRecorder noWipeOnErrorRecorder = buildRecorder(rotator, false); + doThrow(new IOException()).when(rotator).readMatching(any(), anyLong(), anyLong()); + noWipeOnErrorRecorder.getOrLoadPartialLocked(Long.MIN_VALUE, Long.MAX_VALUE); + // Verify that the rotator won't delete files. + verify(rotator, never()).deleteAll(); + } +} diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java index acdc48af55..43c75b742a 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java @@ -2010,18 +2010,18 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } private NetworkStatsRecorder makeTestRecorder(File directory, String prefix, Config config, - boolean includeTags) { + boolean includeTags, boolean wipeOnError) { final NetworkStats.NonMonotonicObserver observer = mock(NetworkStats.NonMonotonicObserver.class); final DropBoxManager dropBox = mock(DropBoxManager.class); return new NetworkStatsRecorder(new FileRotator( directory, prefix, config.rotateAgeMillis, config.deleteAgeMillis), - observer, dropBox, prefix, config.bucketDuration, includeTags); + observer, dropBox, prefix, config.bucketDuration, includeTags, wipeOnError); } private NetworkStatsCollection getLegacyCollection(String prefix, boolean includeTags) { final NetworkStatsRecorder recorder = makeTestRecorder(mLegacyStatsDir, prefix, - mSettings.getDevConfig(), includeTags); + mSettings.getDevConfig(), includeTags, false); return recorder.getOrLoadCompleteLocked(); }