From 2ce66fc8e6621d8e513e731b095f19771bca028b Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 17 May 2022 22:55:35 +0900 Subject: [PATCH 1/5] Add a PersistentInt class. This implements a simple integer written to disk backed by AtomicFile. Bug: 230289468 Test: new unit test Change-Id: I1c73c8ac2429f92153cfba3b006b4374e75e65df Merged-In: I1c73c8ac2429f92153cfba3b006b4374e75e65df (pure cherry-picked from ag/18437645) --- .../com/android/server/net/PersistentInt.java | 108 +++++++++++++++ .../android/server/net/PersistentIntTest.kt | 127 ++++++++++++++++++ 2 files changed, 235 insertions(+) create mode 100644 service-t/src/com/android/server/net/PersistentInt.java create mode 100644 tests/unit/java/com/android/server/net/PersistentIntTest.kt diff --git a/service-t/src/com/android/server/net/PersistentInt.java b/service-t/src/com/android/server/net/PersistentInt.java new file mode 100644 index 0000000000..c212b7727e --- /dev/null +++ b/service-t/src/com/android/server/net/PersistentInt.java @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * 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 android.annotation.NonNull; +import android.annotation.Nullable; +import android.util.AtomicFile; +import android.util.SystemConfigFileCommitEventLogger; + +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; + +/** + * A simple integer backed by an on-disk {@link AtomicFile}. Not thread-safe. + */ +public class PersistentInt { + private final String mPath; + private final AtomicFile mFile; + + /** + * Constructs a new {@code PersistentInt}. The counter is set to 0 if the file does not exist. + * Before returning, the constructor checks that the file is readable and writable. This + * indicates that in the future {@link #get} and {@link #set} are likely to succeed, + * though other events (data corruption, other code deleting the file, etc.) may cause these + * calls to fail in the future. + * + * @param path the path of the file to use. + * @param logger the logger + * @throws IOException the counter could not be read or written + */ + public PersistentInt(@NonNull String path, @Nullable SystemConfigFileCommitEventLogger logger) + throws IOException { + mPath = path; + mFile = new AtomicFile(new File(path), logger); + checkReadWrite(); + } + + private void checkReadWrite() throws IOException { + int value; + try { + value = get(); + } catch (FileNotFoundException e) { + // Counter does not exist. Attempt to initialize to 0. + // Note that we cannot tell here if the file does not exist or if opening it failed, + // because in Java both of those throw FileNotFoundException. + value = 0; + } + set(value); + get(); + // No exceptions? Good. + } + + /** + * Gets the current value. + * + * @return the current value of the counter. + * @throws IOException if reading the value failed. + */ + public int get() throws IOException { + try (FileInputStream fin = mFile.openRead(); + DataInputStream din = new DataInputStream(fin)) { + return din.readInt(); + } + } + + /** + * Sets the current value. + * @param value the value to set + * @throws IOException if writing the value failed. + */ + public void set(int value) throws IOException { + FileOutputStream fout = null; + try { + fout = mFile.startWrite(); + DataOutputStream dout = new DataOutputStream(fout); + dout.writeInt(value); + mFile.finishWrite(fout); + } catch (IOException e) { + if (fout != null) { + mFile.failWrite(fout); + } + throw e; + } + } + + public String getPath() { + return mPath; + } +} diff --git a/tests/unit/java/com/android/server/net/PersistentIntTest.kt b/tests/unit/java/com/android/server/net/PersistentIntTest.kt new file mode 100644 index 0000000000..9509d38549 --- /dev/null +++ b/tests/unit/java/com/android/server/net/PersistentIntTest.kt @@ -0,0 +1,127 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * 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 android.util.SystemConfigFileCommitEventLogger +import com.android.testutils.assertThrows +import org.junit.After +import org.junit.Before +import org.junit.Test +import java.io.File +import java.io.IOException +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.attribute.PosixFilePermission +import java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE +import java.nio.file.attribute.PosixFilePermission.OWNER_READ +import java.nio.file.attribute.PosixFilePermission.OWNER_WRITE +import java.util.Random +import kotlin.test.assertEquals + +class PersistentIntTest { + val tempFilesCreated = mutableSetOf() + lateinit var tempDir: Path + + @Before + fun setUp() { + tempDir = Files.createTempDirectory("tmp.PersistentIntTest.") + } + + @After + fun tearDown() { + var permissions = setOf(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE) + Files.setPosixFilePermissions(tempDir, permissions) + + for (file in tempFilesCreated) { + Files.deleteIfExists(file) + } + Files.delete(tempDir) + } + + @Test + fun testNormalReadWrite() { + // New, initialized to 0. + val pi = createPersistentInt() + assertEquals(0, pi.get()) + pi.set(12345) + assertEquals(12345, pi.get()) + + // Existing. + val pi2 = createPersistentInt(pathOf(pi)) + assertEquals(12345, pi2.get()) + } + + @Test + fun testReadOrWriteFailsInCreate() { + setWritable(tempDir, false) + assertThrows(IOException::class.java) { + createPersistentInt() + } + } + + @Test + fun testReadOrWriteFailsAfterCreate() { + val pi = createPersistentInt() + pi.set(42) + assertEquals(42, pi.get()) + + val path = pathOf(pi) + setReadable(path, false) + assertThrows(IOException::class.java) { pi.get() } + pi.set(77) + + setReadable(path, true) + setWritable(path, false) + setWritable(tempDir, false) // Writing creates a new file+renames, make this fail. + assertThrows(IOException::class.java) { pi.set(99) } + assertEquals(77, pi.get()) + } + + fun addOrRemovePermission(p: Path, permission: PosixFilePermission, add: Boolean) { + val permissions = Files.getPosixFilePermissions(p) + if (add) { + permissions.add(permission) + } else { + permissions.remove(permission) + } + Files.setPosixFilePermissions(p, permissions) + } + + fun setReadable(p: Path, readable: Boolean) { + addOrRemovePermission(p, OWNER_READ, readable) + } + + fun setWritable(p: Path, writable: Boolean) { + addOrRemovePermission(p, OWNER_WRITE, writable) + } + + fun pathOf(pi: PersistentInt): Path { + return File(pi.path).toPath() + } + + fun createPersistentInt(path: Path = randomTempPath()): PersistentInt { + tempFilesCreated.add(path) + return PersistentInt(path.toString(), + SystemConfigFileCommitEventLogger("PersistentIntTest")) + } + + fun randomTempPath(): Path { + return tempDir.resolve(Integer.toHexString(Random().nextInt())).also { + tempFilesCreated.add(it) + } + } +} \ No newline at end of file From a388d4bf06a5d98aac0b26ff35895b3eef98210f Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Fri, 28 Jan 2022 03:38:43 +0000 Subject: [PATCH 2/5] [MS82.1] Support network stats data migration process This includes: 1. Provide NetworkStatsRecorder#importCollectionLocked to allow caller to write a NetworkStatsCollection object to a standalone file. 2. Do not clear the collection in NetworkStatsRecorder# CombiningRewriter, this is safe since pending set will still be cleared during forcePersistLocked. 3. Implement the migration process, including recovery and safety measures. Test: NetworkStatsServiceTest NetworkStatsCollectionTest Test: manual test with adb shell device_config put tethering \ netstats_store_files_in_apexdata true adb shell device_config put tethering \ netstats_import_legacy_target_attempts 1 Bug: 230289468 Change-Id: Ic2002cbfd5a6b3c2226fa8dab22481f8ce656574 Merged-In: Ic2002cbfd5a6b3c2226fa8dab22481f8ce656574 (pure cherry-picked from ag/18367994) --- .../server/net/NetworkStatsRecorder.java | 51 ++- .../server/net/NetworkStatsService.java | 318 +++++++++++++++--- .../server/net/NetworkStatsServiceTest.java | 114 ++++++- 3 files changed, 418 insertions(+), 65 deletions(-) diff --git a/service-t/src/com/android/server/net/NetworkStatsRecorder.java b/service-t/src/com/android/server/net/NetworkStatsRecorder.java index 6f070d79d2..d73e3428e2 100644 --- a/service-t/src/com/android/server/net/NetworkStatsRecorder.java +++ b/service-t/src/com/android/server/net/NetworkStatsRecorder.java @@ -21,6 +21,7 @@ import static android.net.TrafficStats.KB_IN_BYTES; import static android.net.TrafficStats.MB_IN_BYTES; import static android.text.format.DateUtils.YEAR_IN_MILLIS; +import android.annotation.NonNull; import android.net.NetworkIdentitySet; import android.net.NetworkStats; import android.net.NetworkStats.NonMonotonicObserver; @@ -68,7 +69,7 @@ public class NetworkStatsRecorder { private static final String TAG_NETSTATS_DUMP = "netstats_dump"; - /** Dump before deleting in {@link #recoverFromWtf()}. */ + /** Dump before deleting in {@link #recoverAndDeleteData()}. */ private static final boolean DUMP_BEFORE_DELETE = true; private final FileRotator mRotator; @@ -156,6 +157,15 @@ public class NetworkStatsRecorder { return mSinceBoot; } + public long getBucketDuration() { + return mBucketDuration; + } + + @NonNull + public String getCookie() { + return mCookie; + } + /** * Load complete history represented by {@link FileRotator}. Caches * internally as a {@link WeakReference}, and updated with future @@ -189,10 +199,10 @@ public class NetworkStatsRecorder { res.recordCollection(mPending); } catch (IOException e) { Log.wtf(TAG, "problem completely reading network stats", e); - recoverFromWtf(); + recoverAndDeleteData(); } catch (OutOfMemoryError e) { Log.wtf(TAG, "problem completely reading network stats", e); - recoverFromWtf(); + recoverAndDeleteData(); } return res; } @@ -300,10 +310,10 @@ public class NetworkStatsRecorder { mPending.reset(); } catch (IOException e) { Log.wtf(TAG, "problem persisting pending stats", e); - recoverFromWtf(); + recoverAndDeleteData(); } catch (OutOfMemoryError e) { Log.wtf(TAG, "problem persisting pending stats", e); - recoverFromWtf(); + recoverAndDeleteData(); } } } @@ -319,10 +329,10 @@ public class NetworkStatsRecorder { mRotator.rewriteAll(new RemoveUidRewriter(mBucketDuration, uids)); } catch (IOException e) { Log.wtf(TAG, "problem removing UIDs " + Arrays.toString(uids), e); - recoverFromWtf(); + recoverAndDeleteData(); } catch (OutOfMemoryError e) { Log.wtf(TAG, "problem removing UIDs " + Arrays.toString(uids), e); - recoverFromWtf(); + recoverAndDeleteData(); } } @@ -347,8 +357,7 @@ public class NetworkStatsRecorder { /** * Rewriter that will combine current {@link NetworkStatsCollection} values - * with anything read from disk, and write combined set to disk. Clears the - * original {@link NetworkStatsCollection} when finished writing. + * with anything read from disk, and write combined set to disk. */ private static class CombiningRewriter implements FileRotator.Rewriter { private final NetworkStatsCollection mCollection; @@ -375,7 +384,6 @@ public class NetworkStatsRecorder { @Override public void write(OutputStream out) throws IOException { mCollection.write(out); - mCollection.reset(); } } @@ -455,6 +463,23 @@ public class NetworkStatsRecorder { } } + /** + * Import a specified {@link NetworkStatsCollection} instance into this recorder, + * and write it into a standalone file. + * @param collection The target {@link NetworkStatsCollection} instance to be imported. + */ + public void importCollectionLocked(@NonNull NetworkStatsCollection collection) + throws IOException { + if (mRotator != null) { + mRotator.rewriteSingle(new CombiningRewriter(collection), collection.getStartMillis(), + collection.getEndMillis()); + } + + if (mComplete != null) { + throw new IllegalStateException("cannot import data when data already loaded"); + } + } + /** * Rewriter that will remove any histories or persisted data points before the * specified cutoff time, only writing data back when modified. @@ -501,10 +526,10 @@ public class NetworkStatsRecorder { mBucketDuration, cutoffMillis)); } catch (IOException e) { Log.wtf(TAG, "problem importing netstats", e); - recoverFromWtf(); + recoverAndDeleteData(); } catch (OutOfMemoryError e) { Log.wtf(TAG, "problem importing netstats", e); - recoverFromWtf(); + recoverAndDeleteData(); } } @@ -555,7 +580,7 @@ public class NetworkStatsRecorder { * Recover from {@link FileRotator} failure by dumping state to * {@link DropBoxManager} and deleting contents. */ - private void recoverFromWtf() { + void recoverAndDeleteData() { if (DUMP_BEFORE_DELETE) { final ByteArrayOutputStream os = new ByteArrayOutputStream(); try { diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index b2d8b5e410..61e22a11ec 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -67,6 +67,7 @@ import android.annotation.TargetApi; import android.app.AlarmManager; import android.app.PendingIntent; import android.app.usage.NetworkStatsManager; +import android.content.ApexEnvironment; import android.content.BroadcastReceiver; import android.content.ContentResolver; import android.content.Context; @@ -100,6 +101,7 @@ import android.net.TrafficStats; import android.net.UnderlyingNetworkInfo; import android.net.Uri; import android.net.netstats.IUsageCallback; +import android.net.netstats.NetworkStatsDataMigrationUtils; import android.net.netstats.provider.INetworkStatsProvider; import android.net.netstats.provider.INetworkStatsProviderCallback; import android.net.netstats.provider.NetworkStatsProvider; @@ -118,6 +120,7 @@ import android.os.ServiceSpecificException; import android.os.SystemClock; import android.os.Trace; import android.os.UserHandle; +import android.provider.DeviceConfig; import android.provider.Settings; import android.provider.Settings.Global; import android.service.NetworkInterfaceProto; @@ -143,6 +146,7 @@ import com.android.net.module.util.BestClock; import com.android.net.module.util.BinderUtils; import com.android.net.module.util.BpfMap; import com.android.net.module.util.CollectionUtils; +import com.android.net.module.util.DeviceConfigUtils; import com.android.net.module.util.IBpfMap; import com.android.net.module.util.LocationPermissionChecker; import com.android.net.module.util.NetworkStatsUtils; @@ -155,7 +159,9 @@ import java.io.FileDescriptor; import java.io.FileOutputStream; import java.io.IOException; import java.io.PrintWriter; +import java.nio.file.Path; import java.time.Clock; +import java.time.Instant; import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Arrays; @@ -232,6 +238,22 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private static final String STATS_MAP_B_PATH = "/sys/fs/bpf/net_shared/map_netd_stats_map_B"; + /** + * DeviceConfig flag used to indicate whether the files should be stored in the apex data + * directory. + */ + static final String NETSTATS_STORE_FILES_IN_APEXDATA = "netstats_store_files_in_apexdata"; + /** + * DeviceConfig flag is used to indicate whether the legacy files need to be imported, and + * retry count before giving up. Only valid when {@link #NETSTATS_STORE_FILES_IN_APEXDATA} + * set to true. Note that the value gets rollback when the mainline module gets rollback. + */ + static final String NETSTATS_IMPORT_LEGACY_TARGET_ATTEMPTS = + "netstats_import_legacy_target_attempts"; + static final int DEFAULT_NETSTATS_IMPORT_LEGACY_TARGET_ATTEMPTS = 1; + static final String NETSTATS_IMPORT_ATTEMPTS_COUNTER_NAME = "import.attempts"; + static final String NETSTATS_IMPORT_SUCCESS_COUNTER_NAME = "import.successes"; + private final Context mContext; private final NetworkStatsFactory mStatsFactory; private final AlarmManager mAlarmManager; @@ -239,8 +261,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private final NetworkStatsSettings mSettings; private final NetworkStatsObservers mStatsObservers; - private final File mSystemDir; - private final File mBaseDir; + private final File mStatsDir; private final PowerManager.WakeLock mWakeLock; @@ -250,6 +271,12 @@ public class NetworkStatsService extends INetworkStatsService.Stub { protected INetd mNetd; private final AlertObserver mAlertObserver = new AlertObserver(); + // Persistent counters that backed by AtomicFile which stored in the data directory as a file, + // to track attempts/successes count across reboot. Note that these counter values will be + // rollback as the module rollbacks. + private PersistentInt mImportLegacyAttemptsCounter = null; + private PersistentInt mImportLegacySuccessesCounter = null; + @VisibleForTesting public static final String ACTION_NETWORK_STATS_POLL = "com.android.server.action.NETWORK_STATS_POLL"; @@ -405,16 +432,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @NonNull private final BpfInterfaceMapUpdater mInterfaceMapUpdater; - private static @NonNull File getDefaultSystemDir() { - return new File(Environment.getDataDirectory(), "system"); - } - - 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()); @@ -506,8 +523,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { INetd.Stub.asInterface((IBinder) context.getSystemService(Context.NETD_SERVICE)), alarmManager, wakeLock, getDefaultClock(), new DefaultNetworkStatsSettings(), new NetworkStatsFactory(context), - new NetworkStatsObservers(), getDefaultSystemDir(), getDefaultBaseDir(), - new Dependencies()); + new NetworkStatsObservers(), new Dependencies()); return service; } @@ -517,8 +533,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @VisibleForTesting NetworkStatsService(Context context, INetd netd, AlarmManager alarmManager, PowerManager.WakeLock wakeLock, Clock clock, NetworkStatsSettings settings, - NetworkStatsFactory factory, NetworkStatsObservers statsObservers, File systemDir, - File baseDir, @NonNull Dependencies deps) { + NetworkStatsFactory factory, NetworkStatsObservers statsObservers, + @NonNull Dependencies deps) { mContext = Objects.requireNonNull(context, "missing Context"); mNetd = Objects.requireNonNull(netd, "missing Netd"); mAlarmManager = Objects.requireNonNull(alarmManager, "missing AlarmManager"); @@ -527,9 +543,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mWakeLock = Objects.requireNonNull(wakeLock, "missing WakeLock"); mStatsFactory = Objects.requireNonNull(factory, "missing factory"); mStatsObservers = Objects.requireNonNull(statsObservers, "missing NetworkStatsObservers"); - mSystemDir = Objects.requireNonNull(systemDir, "missing systemDir"); - mBaseDir = Objects.requireNonNull(baseDir, "missing baseDir"); mDeps = Objects.requireNonNull(deps, "missing Dependencies"); + mStatsDir = mDeps.getOrCreateStatsDir(); + if (!mStatsDir.exists()) { + throw new IllegalStateException("Persist data directory does not exist: " + mStatsDir); + } final HandlerThread handlerThread = mDeps.makeHandlerThread(); handlerThread.start(); @@ -555,6 +573,79 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // TODO: Move more stuff into dependencies object. @VisibleForTesting public static class Dependencies { + /** + * Get or create the directory that stores the persisted data usage. + */ + @NonNull + public File getOrCreateStatsDir() { + final boolean storeInApexDataDir = getStoreFilesInApexData(); + + final File statsDataDir; + if (storeInApexDataDir) { + final File apexDataDir = ApexEnvironment + .getApexEnvironment(DeviceConfigUtils.TETHERING_MODULE_NAME) + .getDeviceProtectedDataDir(); + statsDataDir = new File(apexDataDir, "netstats"); + + } else { + final File systemDataDir = new File(Environment.getDataDirectory(), "system"); + statsDataDir = new File(systemDataDir, "netstats"); + } + + if (statsDataDir.exists() || statsDataDir.mkdirs()) { + return statsDataDir; + } + throw new IllegalStateException("Cannot write into stats data directory: " + + statsDataDir); + } + + /** + * Get the count of import legacy target attempts. + */ + public int getImportLegacyTargetAttempts() { + return DeviceConfigUtils.getDeviceConfigPropertyInt( + DeviceConfig.NAMESPACE_TETHERING, + NETSTATS_IMPORT_LEGACY_TARGET_ATTEMPTS, + DEFAULT_NETSTATS_IMPORT_LEGACY_TARGET_ATTEMPTS); + } + + /** + * Create the persistent counter that counts total import legacy stats attempts. + */ + public PersistentInt createImportLegacyAttemptsCounter(@NonNull Path path) + throws IOException { + // TODO: Modify PersistentInt to call setStartTime every time a write is made. + // Create and pass a real logger here. + return new PersistentInt(path.toString(), null /* logger */); + } + + /** + * Create the persistent counter that counts total import legacy stats successes. + */ + public PersistentInt createImportLegacySuccessesCounter(@NonNull Path path) + throws IOException { + return new PersistentInt(path.toString(), null /* logger */); + } + + /** + * Get the flag of storing files in the apex data directory. + * @return whether to store files in the apex data directory. + */ + public boolean getStoreFilesInApexData() { + return DeviceConfigUtils.getDeviceConfigPropertyBoolean( + DeviceConfig.NAMESPACE_TETHERING, + NETSTATS_STORE_FILES_IN_APEXDATA, true); + } + + /** + * Read legacy persisted network stats from disk. + */ + @NonNull + public NetworkStatsCollection readPlatformCollection( + @NonNull String prefix, long bucketDuration) throws IOException { + return NetworkStatsDataMigrationUtils.readPlatformCollection(prefix, bucketDuration); + } + /** * Create a HandlerThread to use in NetworkStatsService. */ @@ -697,7 +788,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { updatePersistThresholdsLocked(); - // upgrade any legacy stats, migrating them to rotated files + // upgrade any legacy stats maybeUpgradeLegacyStatsLocked(); // read historical network stats from disk, since policy service @@ -761,7 +852,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { final DropBoxManager dropBox = (DropBoxManager) mContext.getSystemService( Context.DROPBOX_SERVICE); return new NetworkStatsRecorder(new FileRotator( - mBaseDir, prefix, config.rotateAgeMillis, config.deleteAgeMillis), + mStatsDir, prefix, config.rotateAgeMillis, config.deleteAgeMillis), mNonMonotonicObserver, dropBox, prefix, config.bucketDuration, includeTags); } @@ -791,32 +882,151 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mSystemReady = false; } + private static class MigrationInfo { + public final NetworkStatsRecorder recorder; + public NetworkStatsCollection collection; + public boolean imported; + MigrationInfo(@NonNull final NetworkStatsRecorder recorder) { + this.recorder = recorder; + collection = null; + imported = false; + } + } + @GuardedBy("mStatsLock") private void maybeUpgradeLegacyStatsLocked() { - File file; - try { - file = new File(mSystemDir, "netstats.bin"); - if (file.exists()) { - mDevRecorder.importLegacyNetworkLocked(file); - file.delete(); - } - - file = new File(mSystemDir, "netstats_xt.bin"); - if (file.exists()) { - file.delete(); - } - - file = new File(mSystemDir, "netstats_uid.bin"); - if (file.exists()) { - mUidRecorder.importLegacyUidLocked(file); - mUidTagRecorder.importLegacyUidLocked(file); - file.delete(); - } - } catch (IOException e) { - Log.wtf(TAG, "problem during legacy upgrade", e); - } catch (OutOfMemoryError e) { - Log.wtf(TAG, "problem during legacy upgrade", e); + final boolean storeFilesInApexData = mDeps.getStoreFilesInApexData(); + if (!storeFilesInApexData) { + return; } + try { + mImportLegacyAttemptsCounter = mDeps.createImportLegacyAttemptsCounter( + mStatsDir.toPath().resolve(NETSTATS_IMPORT_ATTEMPTS_COUNTER_NAME)); + mImportLegacySuccessesCounter = mDeps.createImportLegacySuccessesCounter( + mStatsDir.toPath().resolve(NETSTATS_IMPORT_SUCCESS_COUNTER_NAME)); + } catch (IOException e) { + Log.wtf(TAG, "Failed to create persistent counters, skip.", e); + return; + } + + final int targetAttempts = mDeps.getImportLegacyTargetAttempts(); + final int attempts; + try { + attempts = mImportLegacyAttemptsCounter.get(); + } catch (IOException e) { + Log.wtf(TAG, "Failed to read attempts counter, skip.", e); + return; + } + if (attempts >= targetAttempts) return; + + Log.i(TAG, "Starting import : attempts " + attempts + "/" + targetAttempts); + + final List migrations = List.of( + new MigrationInfo(mDevRecorder), new MigrationInfo(mXtRecorder), + new MigrationInfo(mUidRecorder), new MigrationInfo(mUidTagRecorder) + ); + long migrationEndTime = Long.MIN_VALUE; + try { + // First, read all legacy collections. This is OEM code and it can throw. Don't + // commit any data to disk until all are read. + for (final MigrationInfo migration : migrations) { + migration.collection = readPlatformCollectionForRecorder(migration.recorder); + } + + // Find the latest end time. + for (final MigrationInfo migration : migrations) { + final long migrationEnd = migration.collection.getEndMillis(); + if (migrationEnd > migrationEndTime) migrationEndTime = migrationEnd; + } + + // Reading all collections from legacy data has succeeded. At this point it is + // safe to start overwriting the files on disk. The next step is to remove all + // data in the new location that overlaps with imported data. This ensures that + // any data in the new location that was created by a previous failed import is + // ignored. After that, write the imported data into the recorder. The code + // below can still possibly throw (disk error or OutOfMemory for example), but + // does not depend on code from non-mainline code. + Log.i(TAG, "Rewriting data with imported collections with cutoff " + + Instant.ofEpochMilli(migrationEndTime)); + for (final MigrationInfo migration : migrations) { + migration.imported = true; + migration.recorder.removeDataBefore(migrationEndTime); + if (migration.collection.isEmpty()) continue; + migration.recorder.importCollectionLocked(migration.collection); + } + + Log.i(TAG, "Successfully imported platform collections"); + } catch (Throwable e) { + // The code above calls OEM code that may behave differently across devices. + // It can throw any exception including RuntimeExceptions and + // OutOfMemoryErrors. Try to recover anyway. + Log.wtf(TAG, "Platform data import failed. Remaining tries " + + (targetAttempts - attempts), e); + + // Failed this time around : try again next time unless we're out of tries. + try { + mImportLegacyAttemptsCounter.set(attempts + 1); + } catch (IOException ex) { + Log.wtf(TAG, "Failed to update attempts counter.", ex); + } + + // Try to remove any data from the failed import. + if (migrationEndTime > Long.MIN_VALUE) { + try { + for (final MigrationInfo migration : migrations) { + if (migration.imported) { + migration.recorder.removeDataBefore(migrationEndTime); + } + } + } catch (Throwable f) { + // If rollback still throws, there isn't much left to do. Try nuking + // all data, since that's the last stop. If nuking still throws, the + // framework will reboot, and if there are remaining tries, the migration + // process will retry, which is fine because it's idempotent. + for (final MigrationInfo migration : migrations) { + migration.recorder.recoverAndDeleteData(); + } + } + } + + return; + } + + // Success ! No need to import again next time. + try { + mImportLegacyAttemptsCounter.set(targetAttempts); + // The successes counter is only for debugging. Hence, the synchronization + // between these two counters are not very critical. + final int successCount = mImportLegacySuccessesCounter.get(); + mImportLegacySuccessesCounter.set(successCount + 1); + } catch (IOException e) { + Log.wtf(TAG, "Succeed but failed to update counters.", e); + } + } + + @GuardedBy("mStatsLock") + @NonNull + private NetworkStatsCollection readPlatformCollectionForRecorder( + @NonNull final NetworkStatsRecorder rec) throws IOException { + final String prefix = rec.getCookie(); + Log.i(TAG, "Importing platform collection for prefix " + prefix); + final NetworkStatsCollection collection = Objects.requireNonNull( + mDeps.readPlatformCollection(prefix, rec.getBucketDuration()), + "Imported platform collection for prefix " + prefix + " must not be null"); + + final long bootTimestamp = System.currentTimeMillis() - SystemClock.elapsedRealtime(); + if (!collection.isEmpty() && bootTimestamp < collection.getStartMillis()) { + throw new IllegalArgumentException("Platform collection for prefix " + prefix + + " contains data that could not possibly come from the previous boot " + + "(start timestamp = " + Instant.ofEpochMilli(collection.getStartMillis()) + + ", last booted at " + Instant.ofEpochMilli(bootTimestamp)); + } + + Log.i(TAG, "Successfully read platform collection spanning from " + // Instant uses ISO-8601 for toString() + + Instant.ofEpochMilli(collection.getStartMillis()).toString() + " to " + + Instant.ofEpochMilli(collection.getEndMillis()).toString()); + return collection; } /** @@ -2102,10 +2312,32 @@ public class NetworkStatsService extends INetworkStatsService.Stub { return; } + pw.println("Directory:"); + pw.increaseIndent(); + pw.println(mStatsDir); + pw.decreaseIndent(); + pw.println("Configs:"); pw.increaseIndent(); pw.print(NETSTATS_COMBINE_SUBTYPE_ENABLED, mSettings.getCombineSubtypeEnabled()); pw.println(); + pw.print(NETSTATS_STORE_FILES_IN_APEXDATA, mDeps.getStoreFilesInApexData()); + pw.println(); + pw.print(NETSTATS_IMPORT_LEGACY_TARGET_ATTEMPTS, mDeps.getImportLegacyTargetAttempts()); + pw.println(); + if (mDeps.getStoreFilesInApexData()) { + try { + pw.print("platform legacy stats import attempts count", + mImportLegacyAttemptsCounter.get()); + pw.println(); + pw.print("platform legacy stats import successes count", + mImportLegacySuccessesCounter.get()); + pw.println(); + } catch (IOException e) { + pw.println("(failed to dump platform legacy stats import counters)"); + } + } + pw.decreaseIndent(); pw.println("Active interfaces:"); diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java index ceeb997ff6..24731bfa41 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java @@ -94,8 +94,10 @@ import android.net.INetworkStatsSession; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; +import android.net.NetworkIdentity; import android.net.NetworkStateSnapshot; import android.net.NetworkStats; +import android.net.NetworkStatsCollection; import android.net.NetworkStatsHistory; import android.net.NetworkTemplate; import android.net.TelephonyNetworkSpecifier; @@ -143,9 +145,11 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.io.File; +import java.nio.file.Path; import java.time.Clock; import java.time.ZoneOffset; import java.util.Objects; +import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; @@ -220,6 +224,12 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { private ContentObserver mContentObserver; private Handler mHandler; private TetheringManager.TetheringEventCallback mTetheringEventCallback; + private NetworkStatsCollection mPlatformNetworkStatsCollection = + new NetworkStatsCollection(30 * HOUR_IN_MILLIS); + private boolean mStoreFilesInApexData = false; + private int mImportLegacyTargetAttempts = 0; + private @Mock PersistentInt mImportLegacyAttemptsCounter; + private @Mock PersistentInt mImportLegacySuccessesCounter; private class MockContext extends BroadcastInterceptingContext { private final Context mBaseContext; @@ -295,8 +305,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { mHandlerThread = new HandlerThread("HandlerThread"); final NetworkStatsService.Dependencies deps = makeDependencies(); mService = new NetworkStatsService(mServiceContext, mNetd, mAlarmManager, wakeLock, - mClock, mSettings, mStatsFactory, new NetworkStatsObservers(), mStatsDir, - getBaseDir(mStatsDir), deps); + mClock, mSettings, mStatsFactory, new NetworkStatsObservers(), deps); mElapsedRealtime = 0L; @@ -338,6 +347,39 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { @NonNull private NetworkStatsService.Dependencies makeDependencies() { return new NetworkStatsService.Dependencies() { + @Override + public File getOrCreateStatsDir() { + return mStatsDir; + } + + @Override + public boolean getStoreFilesInApexData() { + return mStoreFilesInApexData; + } + + @Override + public int getImportLegacyTargetAttempts() { + return mImportLegacyTargetAttempts; + } + + @Override + public PersistentInt createImportLegacyAttemptsCounter( + @androidx.annotation.NonNull Path path) { + return mImportLegacyAttemptsCounter; + } + + @Override + public PersistentInt createImportLegacySuccessesCounter( + @androidx.annotation.NonNull Path path) { + return mImportLegacySuccessesCounter; + } + + @Override + public NetworkStatsCollection readPlatformCollection( + @NonNull String prefix, long bucketDuration) { + return mPlatformNetworkStatsCollection; + } + @Override public HandlerThread makeHandlerThread() { return mHandlerThread; @@ -1704,10 +1746,65 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { assertNetworkTotal(sTemplateImsi1, 0L, 0L, 0L, 0L, 0); } - private static File getBaseDir(File statsDir) { - File baseDir = new File(statsDir, "netstats"); - baseDir.mkdirs(); - return baseDir; + /** + * Verify the service will perform data migration process can be controlled by the device flag. + */ + @Test + public void testDataMigration() throws Exception { + assertStatsFilesExist(false); + expectDefaultSettings(); + final long bucketDuration = 30 * HOUR_IN_MILLIS; + final NetworkIdentity ident = new NetworkIdentity.Builder() + .setType(TYPE_MOBILE) + .setMetered(true) + .setSubscriberId(IMSI_1).build(); + final NetworkStatsCollection.Key key = new NetworkStatsCollection.Key( + Set.of(ident), UID_ALL, SET_FOREGROUND, 0x0 /* tag */); + final NetworkStatsHistory history = new NetworkStatsHistory.Builder(bucketDuration, 0) + .addEntry(new NetworkStatsHistory.Entry(0, 10, 31, 3, 50, 5, 1)).build(); + + // Mock mobile traffic which will be reported by + // NetworkStatsDataMigrationUtils and verify it won't be absorbed if the flag is not set. + // TODO: Also mock UID traffic when service queries with PREFIX_UID. And + // verify with assertUidTotal. + mPlatformNetworkStatsCollection = new NetworkStatsCollection.Builder(bucketDuration) + .addEntry(key, history).build(); + mStoreFilesInApexData = true; + mImportLegacyTargetAttempts = 0; + mServiceContext.sendBroadcast(new Intent(Intent.ACTION_SHUTDOWN)); + assertStatsFilesExist(false); + + // Mock zero usage and boot through serviceReady(), verify there is no imported data. + expectDefaultSettings(); + expectNetworkStatsUidDetail(buildEmptyStats()); + expectSystemReady(); + mService.systemReady(); + assertStatsFilesExist(false); + + // Set the flag and reboot, verify the imported data is not there until next boot. + mImportLegacyTargetAttempts = 3; + mServiceContext.sendBroadcast(new Intent(Intent.ACTION_SHUTDOWN)); + assertStatsFilesExist(false); + + // Boot through systemReady() again. + expectDefaultSettings(); + expectNetworkStatsUidDetail(buildEmptyStats()); + expectSystemReady(); + mService.systemReady(); + + // After systemReady(), the service should have historical stats loaded again. + // Thus, verify + // 1. The stats are absorbed by the recorder. + // 2. The imported data are persisted. + // 3. The attempts count is set to target attempts count to indicate a successful + // migration. + assertNetworkTotal(sTemplateImsi1, 31L, 3L, 50L, 5L, 1); + assertStatsFilesExist(true); + verify(mImportLegacyAttemptsCounter).set(3); + verify(mImportLegacySuccessesCounter).set(1); + + // TODO: Verify upgrading with Exception won't damege original data and + // will decrease the retry counter by 1. } private void assertNetworkTotal(NetworkTemplate template, long rxBytes, long rxPackets, @@ -1816,11 +1913,10 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } private void assertStatsFilesExist(boolean exist) { - final File basePath = new File(mStatsDir, "netstats"); if (exist) { - assertTrue(basePath.list().length > 0); + assertTrue(mStatsDir.list().length > 0); } else { - assertTrue(basePath.list().length == 0); + assertTrue(mStatsDir.list().length == 0); } } From c62261f14035d391a229c03a3cc2b9dfd25b6778 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Tue, 17 May 2022 18:23:00 +0900 Subject: [PATCH 3/5] Ensure NetworkStats migrated snapshot is identical Read migration snapshot both from the platform API and the legacy method, and Log.wtf if they are different. Use the legacy method data if they are. This ensures that using the migration API is a no-op, and that errors are reported (through Log.wtf) if it would not have been if used as-is. Test: NetworkStatsServiceTest Bug: 230289468 Change-Id: I857ad18183d63d1aa16e89f89eb24009648720a2 Merged-In: I857ad18183d63d1aa16e89f89eb24009648720a2 (pure cherry-picked from ag/18452103) --- .../src/android/net/NetworkStatsHistory.java | 20 +++ .../server/net/NetworkStatsService.java | 169 ++++++++++++++++-- .../server/net/NetworkStatsServiceTest.java | 121 +++++++++---- 3 files changed, 267 insertions(+), 43 deletions(-) diff --git a/framework-t/src/android/net/NetworkStatsHistory.java b/framework-t/src/android/net/NetworkStatsHistory.java index b45d44dec1..60dad905b2 100644 --- a/framework-t/src/android/net/NetworkStatsHistory.java +++ b/framework-t/src/android/net/NetworkStatsHistory.java @@ -32,6 +32,7 @@ import static android.text.format.DateUtils.SECOND_IN_MILLIS; import static com.android.net.module.util.NetworkStatsUtils.multiplySafeByRational; import android.annotation.NonNull; +import android.annotation.Nullable; import android.annotation.SystemApi; import android.compat.annotation.UnsupportedAppUsage; import android.os.Build; @@ -949,6 +950,25 @@ public final class NetworkStatsHistory implements Parcelable { return writer.toString(); } + /** + * Same as "equals", but not actually called equals as this would affect public API behavior. + * @hide + */ + @Nullable + public boolean isSameAs(NetworkStatsHistory other) { + return bucketCount == other.bucketCount + && Arrays.equals(bucketStart, other.bucketStart) + // Don't check activeTime since it can change on import due to the importer using + // recordHistory. It's also not exposed by the APIs or present in dumpsys or + // toString(). + && Arrays.equals(rxBytes, other.rxBytes) + && Arrays.equals(rxPackets, other.rxPackets) + && Arrays.equals(txBytes, other.txBytes) + && Arrays.equals(txPackets, other.txPackets) + && Arrays.equals(operations, other.operations) + && totalBytes == other.totalBytes; + } + @UnsupportedAppUsage public static final @android.annotation.NonNull Creator CREATOR = new Creator() { @Override diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 61e22a11ec..892c0b6f1c 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -76,6 +76,7 @@ import android.content.IntentFilter; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.database.ContentObserver; +import android.net.ConnectivityManager; import android.net.DataUsageRequest; import android.net.INetd; import android.net.INetworkStatsService; @@ -573,6 +574,15 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // TODO: Move more stuff into dependencies object. @VisibleForTesting public static class Dependencies { + /** + * Get legacy platform stats directory. + */ + @NonNull + public File getLegacyStatsDir() { + final File systemDataDir = new File(Environment.getDataDirectory(), "system"); + return new File(systemDataDir, "netstats"); + } + /** * Get or create the directory that stores the persisted data usage. */ @@ -588,8 +598,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { statsDataDir = new File(apexDataDir, "netstats"); } else { - final File systemDataDir = new File(Environment.getDataDirectory(), "system"); - statsDataDir = new File(systemDataDir, "netstats"); + statsDataDir = getLegacyStatsDir(); } if (statsDataDir.exists() || statsDataDir.mkdirs()) { @@ -781,10 +790,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mSystemReady = true; // create data recorders along with historical rotators - mDevRecorder = buildRecorder(PREFIX_DEV, mSettings.getDevConfig(), false); - mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false); - mUidRecorder = buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false); - mUidTagRecorder = buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true); + mDevRecorder = buildRecorder(PREFIX_DEV, mSettings.getDevConfig(), false, mStatsDir); + mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, mStatsDir); + mUidRecorder = buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, mStatsDir); + mUidTagRecorder = buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, + mStatsDir); updatePersistThresholdsLocked(); @@ -848,11 +858,12 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } private NetworkStatsRecorder buildRecorder( - String prefix, NetworkStatsSettings.Config config, boolean includeTags) { + String prefix, NetworkStatsSettings.Config config, boolean includeTags, + File baseDir) { final DropBoxManager dropBox = (DropBoxManager) mContext.getSystemService( Context.DROPBOX_SERVICE); return new NetworkStatsRecorder(new FileRotator( - mStatsDir, prefix, config.rotateAgeMillis, config.deleteAgeMillis), + baseDir, prefix, config.rotateAgeMillis, config.deleteAgeMillis), mNonMonotonicObserver, dropBox, prefix, config.bucketDuration, includeTags); } @@ -921,16 +932,59 @@ public class NetworkStatsService extends INetworkStatsService.Stub { Log.i(TAG, "Starting import : attempts " + attempts + "/" + targetAttempts); - final List migrations = List.of( + final MigrationInfo[] migrations = new MigrationInfo[]{ new MigrationInfo(mDevRecorder), new MigrationInfo(mXtRecorder), new MigrationInfo(mUidRecorder), new MigrationInfo(mUidTagRecorder) - ); + }; + + // Legacy directories will be created by recorders if they do not exist + final File legacyBaseDir = mDeps.getLegacyStatsDir(); + final NetworkStatsRecorder[] 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) + }; + long migrationEndTime = Long.MIN_VALUE; + boolean endedWithFallback = false; try { // First, read all legacy collections. This is OEM code and it can throw. Don't // commit any data to disk until all are read. - for (final MigrationInfo migration : migrations) { + for (int i = 0; i < migrations.length; i++) { + final MigrationInfo migration = migrations[i]; migration.collection = readPlatformCollectionForRecorder(migration.recorder); + + // Also read the collection with legacy method + final NetworkStatsRecorder legacyRecorder = legacyRecorders[i]; + + final NetworkStatsCollection legacyStats; + try { + legacyStats = legacyRecorder.getOrLoadCompleteLocked(); + } catch (Throwable e) { + Log.wtf(TAG, "Failed to read stats with legacy method", e); + // Newer stats will be used here; that's the only thing that is usable + continue; + } + + String errMsg; + Throwable exception = null; + try { + errMsg = compareStats(migration.collection, legacyStats); + } catch (Throwable e) { + errMsg = "Failed to compare migrated stats with all stats"; + exception = e; + } + + if (errMsg != null) { + Log.wtf(TAG, "NetworkStats import for migration " + i + + " returned invalid data: " + errMsg, exception); + // Fall back to legacy stats for this boot. The stats for old data will be + // re-imported again on next boot until they succeed the import. This is fine + // since every import clears the previous stats for the imported timespan. + migration.collection = legacyStats; + endedWithFallback = true; + } } // Find the latest end time. @@ -955,7 +1009,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { migration.recorder.importCollectionLocked(migration.collection); } - Log.i(TAG, "Successfully imported platform collections"); + if (endedWithFallback) { + Log.wtf(TAG, "Imported platform collections with legacy fallback"); + } else { + Log.i(TAG, "Successfully imported platform collections"); + } } catch (Throwable e) { // The code above calls OEM code that may behave differently across devices. // It can throw any exception including RuntimeExceptions and @@ -1004,6 +1062,93 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } + private static String str(NetworkStatsCollection.Key key) { + StringBuilder sb = new StringBuilder() + .append(key.ident.toString()) + .append(" uid=").append(key.uid); + if (key.set != SET_FOREGROUND) { + sb.append(" set=").append(key.set); + } + if (key.tag != 0) { + sb.append(" tag=").append(key.tag); + } + return sb.toString(); + } + + // The importer will modify some keys when importing them. + // In order to keep the comparison code simple, add such special cases here and simply + // ignore them. This should not impact fidelity much because the start/end checks and the total + // bytes check still need to pass. + private static boolean couldKeyChangeOnImport(NetworkStatsCollection.Key key) { + if (key.ident.isEmpty()) return false; + final NetworkIdentity firstIdent = key.ident.iterator().next(); + + // Non-mobile network with non-empty RAT type. + // This combination is invalid and the NetworkIdentity.Builder will throw if it is passed + // in, but it looks like it was previously possible to persist it to disk. The importer sets + // the RAT type to NETWORK_TYPE_ALL. + if (firstIdent.getType() != ConnectivityManager.TYPE_MOBILE + && firstIdent.getRatType() != NetworkTemplate.NETWORK_TYPE_ALL) { + return true; + } + + return false; + } + + @Nullable + private static String compareStats( + NetworkStatsCollection migrated, NetworkStatsCollection legacy) { + final Map migEntries = + migrated.getEntries(); + final Map legEntries = legacy.getEntries(); + + final ArraySet unmatchedLegKeys = + new ArraySet<>(legEntries.keySet()); + + for (NetworkStatsCollection.Key legKey : legEntries.keySet()) { + final NetworkStatsHistory legHistory = legEntries.get(legKey); + final NetworkStatsHistory migHistory = migEntries.get(legKey); + + if (migHistory == null && couldKeyChangeOnImport(legKey)) { + unmatchedLegKeys.remove(legKey); + continue; + } + + if (migHistory == null) { + return "Missing migrated history for legacy key " + str(legKey) + + ", legacy history was " + legHistory; + } + if (!migHistory.isSameAs(legHistory)) { + return "Difference in history for key " + legKey + "; legacy history " + legHistory + + ", migrated history " + migHistory; + } + unmatchedLegKeys.remove(legKey); + } + + if (!unmatchedLegKeys.isEmpty()) { + final NetworkStatsHistory first = legEntries.get(unmatchedLegKeys.valueAt(0)); + return "Found unmatched legacy keys: count=" + unmatchedLegKeys.size() + + ", first unmatched collection " + first; + } + + if (migrated.getStartMillis() != legacy.getStartMillis() + || migrated.getEndMillis() != legacy.getEndMillis()) { + return "Start / end of the collections " + + migrated.getStartMillis() + "/" + legacy.getStartMillis() + " and " + + migrated.getEndMillis() + "/" + legacy.getEndMillis() + + " don't match"; + } + + if (migrated.getTotalBytes() != legacy.getTotalBytes()) { + return "Total bytes " + migrated.getTotalBytes() + " and " + legacy.getTotalBytes() + + " don't match for collections with start/end " + + migrated.getStartMillis() + + "/" + legacy.getStartMillis(); + } + + return null; + } + @GuardedBy("mStatsLock") @NonNull private NetworkStatsCollection readPlatformCollectionForRecorder( diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java index 24731bfa41..f1820b3acb 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java @@ -18,6 +18,7 @@ package com.android.server.net; import static android.Manifest.permission.READ_NETWORK_USAGE_HISTORY; import static android.Manifest.permission.UPDATE_DEVICE_STATS; +import static android.app.usage.NetworkStatsManager.PREFIX_DEV; import static android.content.Intent.ACTION_UID_REMOVED; import static android.content.Intent.EXTRA_UID; import static android.content.pm.PackageManager.PERMISSION_DENIED; @@ -56,6 +57,9 @@ import static android.net.NetworkTemplate.buildTemplateWifiWildcard; import static android.net.TrafficStats.MB_IN_BYTES; import static android.net.TrafficStats.UID_REMOVED; import static android.net.TrafficStats.UID_TETHERING; +import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_UID; +import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_UID_TAG; +import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_XT; import static android.text.format.DateUtils.DAY_IN_MILLIS; import static android.text.format.DateUtils.HOUR_IN_MILLIS; import static android.text.format.DateUtils.MINUTE_IN_MILLIS; @@ -77,6 +81,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; @@ -94,7 +99,6 @@ import android.net.INetworkStatsSession; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; -import android.net.NetworkIdentity; import android.net.NetworkStateSnapshot; import android.net.NetworkStats; import android.net.NetworkStatsCollection; @@ -106,6 +110,7 @@ import android.net.TetheringManager; import android.net.UnderlyingNetworkInfo; import android.net.netstats.provider.INetworkStatsProviderCallback; import android.net.wifi.WifiInfo; +import android.os.DropBoxManager; import android.os.Handler; import android.os.HandlerThread; import android.os.IBinder; @@ -114,11 +119,13 @@ import android.os.SimpleClock; import android.provider.Settings; import android.system.ErrnoException; import android.telephony.TelephonyManager; +import android.util.ArrayMap; import androidx.annotation.Nullable; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; +import com.android.internal.util.FileRotator; import com.android.internal.util.test.BroadcastInterceptingContext; import com.android.net.module.util.IBpfMap; import com.android.net.module.util.LocationPermissionChecker; @@ -133,6 +140,16 @@ import com.android.testutils.HandlerUtils; import com.android.testutils.TestBpfMap; import com.android.testutils.TestableNetworkStatsProviderBinder; +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Clock; +import java.time.ZoneOffset; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; + import libcore.testing.io.TestIoUtils; import org.junit.After; @@ -144,15 +161,6 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import java.io.File; -import java.nio.file.Path; -import java.time.Clock; -import java.time.ZoneOffset; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.Executor; -import java.util.concurrent.atomic.AtomicBoolean; - /** * Tests for {@link NetworkStatsService}. * @@ -191,6 +199,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { private long mElapsedRealtime; private File mStatsDir; + private File mLegacyStatsDir; private MockContext mServiceContext; private @Mock TelephonyManager mTelephonyManager; private static @Mock WifiInfo sWifiInfo; @@ -224,8 +233,8 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { private ContentObserver mContentObserver; private Handler mHandler; private TetheringManager.TetheringEventCallback mTetheringEventCallback; - private NetworkStatsCollection mPlatformNetworkStatsCollection = - new NetworkStatsCollection(30 * HOUR_IN_MILLIS); + private Map mPlatformNetworkStatsCollection = + new ArrayMap(); private boolean mStoreFilesInApexData = false; private int mImportLegacyTargetAttempts = 0; private @Mock PersistentInt mImportLegacyAttemptsCounter; @@ -296,6 +305,8 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { any(), any(), anyInt(), anyBoolean(), any())).thenReturn(true); when(sWifiInfo.getNetworkKey()).thenReturn(TEST_WIFI_NETWORK_KEY); mStatsDir = TestIoUtils.createTemporaryDirectory(getClass().getSimpleName()); + mLegacyStatsDir = TestIoUtils.createTemporaryDirectory( + getClass().getSimpleName() + "-legacy"); PowerManager powerManager = (PowerManager) mServiceContext.getSystemService( Context.POWER_SERVICE); @@ -347,6 +358,11 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { @NonNull private NetworkStatsService.Dependencies makeDependencies() { return new NetworkStatsService.Dependencies() { + @Override + public File getLegacyStatsDir() { + return mLegacyStatsDir; + } + @Override public File getOrCreateStatsDir() { return mStatsDir; @@ -377,7 +393,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { @Override public NetworkStatsCollection readPlatformCollection( @NonNull String prefix, long bucketDuration) { - return mPlatformNetworkStatsCollection; + return mPlatformNetworkStatsCollection.get(prefix); } @Override @@ -1753,27 +1769,53 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { public void testDataMigration() throws Exception { assertStatsFilesExist(false); expectDefaultSettings(); - final long bucketDuration = 30 * HOUR_IN_MILLIS; - final NetworkIdentity ident = new NetworkIdentity.Builder() - .setType(TYPE_MOBILE) - .setMetered(true) - .setSubscriberId(IMSI_1).build(); - final NetworkStatsCollection.Key key = new NetworkStatsCollection.Key( - Set.of(ident), UID_ALL, SET_FOREGROUND, 0x0 /* tag */); - final NetworkStatsHistory history = new NetworkStatsHistory.Builder(bucketDuration, 0) - .addEntry(new NetworkStatsHistory.Entry(0, 10, 31, 3, 50, 5, 1)).build(); - // Mock mobile traffic which will be reported by - // NetworkStatsDataMigrationUtils and verify it won't be absorbed if the flag is not set. - // TODO: Also mock UID traffic when service queries with PREFIX_UID. And - // verify with assertUidTotal. - mPlatformNetworkStatsCollection = new NetworkStatsCollection.Builder(bucketDuration) - .addEntry(key, history).build(); - mStoreFilesInApexData = true; - mImportLegacyTargetAttempts = 0; + NetworkStateSnapshot[] states = new NetworkStateSnapshot[] {buildWifiState()}; + + mService.notifyNetworkStatus(NETWORKS_WIFI, states, getActiveIface(states), + new UnderlyingNetworkInfo[0]); + + // modify some number on wifi, and trigger poll event + incrementCurrentTime(HOUR_IN_MILLIS); + // expectDefaultSettings(); + expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) + .insertEntry(TEST_IFACE, 1024L, 8L, 2048L, 16L)); + expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 2) + .insertEntry(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 512L, 4L, 256L, 2L, 0L) + .insertEntry(TEST_IFACE, UID_RED, SET_DEFAULT, 0xFAAD, 256L, 2L, 128L, 1L, 0L) + .insertEntry(TEST_IFACE, UID_RED, SET_FOREGROUND, TAG_NONE, 512L, 4L, 256L, 2L, 0L) + .insertEntry(TEST_IFACE, UID_RED, SET_FOREGROUND, 0xFAAD, 256L, 2L, 128L, 1L, 0L) + .insertEntry(TEST_IFACE, UID_BLUE, SET_DEFAULT, TAG_NONE, 128L, 1L, 128L, 1L, 0L)); + + mService.noteUidForeground(UID_RED, false); + verify(mUidCounterSetMap, never()).deleteEntry(any()); + mService.incrementOperationCount(UID_RED, 0xFAAD, 4); + mService.noteUidForeground(UID_RED, true); + verify(mUidCounterSetMap).updateEntry( + eq(new U32(UID_RED)), eq(new U8((short) SET_FOREGROUND))); + mService.incrementOperationCount(UID_RED, 0xFAAD, 6); + + forcePollAndWaitForIdle(); + // Simulate shutdown to force persisting data mServiceContext.sendBroadcast(new Intent(Intent.ACTION_SHUTDOWN)); + assertStatsFilesExist(true); + + // Move the files to the legacy directory to simulate an import from old data + for (File f : mStatsDir.listFiles()) { + Files.move(f.toPath(), mLegacyStatsDir.toPath().resolve(f.getName())); + } assertStatsFilesExist(false); + // Fetch the stats from the legacy files and set platform stats collection to be identical + mPlatformNetworkStatsCollection.put(PREFIX_DEV, + getLegacyCollection(PREFIX_DEV, false /* includeTags */)); + mPlatformNetworkStatsCollection.put(PREFIX_XT, + getLegacyCollection(PREFIX_XT, false /* includeTags */)); + mPlatformNetworkStatsCollection.put(PREFIX_UID, + getLegacyCollection(PREFIX_UID, false /* includeTags */)); + mPlatformNetworkStatsCollection.put(PREFIX_UID_TAG, + getLegacyCollection(PREFIX_UID_TAG, true /* includeTags */)); + // Mock zero usage and boot through serviceReady(), verify there is no imported data. expectDefaultSettings(); expectNetworkStatsUidDetail(buildEmptyStats()); @@ -1782,6 +1824,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { assertStatsFilesExist(false); // Set the flag and reboot, verify the imported data is not there until next boot. + mStoreFilesInApexData = true; mImportLegacyTargetAttempts = 3; mServiceContext.sendBroadcast(new Intent(Intent.ACTION_SHUTDOWN)); assertStatsFilesExist(false); @@ -1798,7 +1841,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { // 2. The imported data are persisted. // 3. The attempts count is set to target attempts count to indicate a successful // migration. - assertNetworkTotal(sTemplateImsi1, 31L, 3L, 50L, 5L, 1); + assertNetworkTotal(sTemplateWifi, 1024L, 8L, 2048L, 16L, 0); assertStatsFilesExist(true); verify(mImportLegacyAttemptsCounter).set(3); verify(mImportLegacySuccessesCounter).set(1); @@ -1807,6 +1850,22 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { // will decrease the retry counter by 1. } + private NetworkStatsRecorder makeTestRecorder(File directory, String prefix, Config config, + boolean includeTags) { + 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); + } + + private NetworkStatsCollection getLegacyCollection(String prefix, boolean includeTags) { + final NetworkStatsRecorder recorder = makeTestRecorder(mLegacyStatsDir, PREFIX_DEV, + mSettings.getDevConfig(), includeTags); + return recorder.getOrLoadCompleteLocked(); + } + private void assertNetworkTotal(NetworkTemplate template, long rxBytes, long rxPackets, long txBytes, long txPackets, int operations) throws Exception { assertNetworkTotal(template, Long.MIN_VALUE, Long.MAX_VALUE, rxBytes, rxPackets, txBytes, From ec2fbb7159c6eeb4e9dd90f2af05bc4cfb58ae85 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Wed, 18 May 2022 17:32:37 +0900 Subject: [PATCH 4/5] Don't clobber existing history entries. Currently, adding a history to a NetworkStatsCollection.Builder will overwrite any history that was previously passed in with the same key. This breaks the importer (which is the primary/only caller of this code), because the importer re-uses the same NetworkStatsCollection object to import multiple files. Instead, simply add any passed-in entries after the ones that were already there. Require the caller to pass in entries in order, because NetworkStatsHistory internally assumes that entris are always sorted. Bug: 230289468 Test: manually verified this unbreaks the importer Change-Id: Ic8647ff28fca78d579d5f759f96a864877f8158b Merged-In: Ic8647ff28fca78d579d5f759f96a864877f8158b (pure cherry-picked from ag/18453213) --- .../android/net/NetworkStatsCollection.java | 17 +++++++++- .../src/android/net/NetworkStatsHistory.java | 32 ++++++++++++++++++- .../net/netstats/NetworkStatsHistoryTest.kt | 15 +++++++-- .../NetworkStatsDataMigrationUtilsTest.kt | 8 ----- 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/framework-t/src/android/net/NetworkStatsCollection.java b/framework-t/src/android/net/NetworkStatsCollection.java index 29ea772bc3..6a1d2dd222 100644 --- a/framework-t/src/android/net/NetworkStatsCollection.java +++ b/framework-t/src/android/net/NetworkStatsCollection.java @@ -865,6 +865,9 @@ public class NetworkStatsCollection implements FileRotator.Reader, FileRotator.W * Add association of the history with the specified key in this map. * * @param key The object used to identify a network, see {@link Key}. + * If history already exists for this key, then the passed-in history is appended + * to the previously-passed in history. The caller must ensure that the history + * passed-in timestamps are greater than all previously-passed-in timestamps. * @param history {@link NetworkStatsHistory} instance associated to the given {@link Key}. * @return The builder object. */ @@ -874,9 +877,21 @@ public class NetworkStatsCollection implements FileRotator.Reader, FileRotator.W Objects.requireNonNull(key); Objects.requireNonNull(history); final List historyEntries = history.getEntries(); + final NetworkStatsHistory existing = mEntries.get(key); + final int size = historyEntries.size() + ((existing != null) ? existing.size() : 0); final NetworkStatsHistory.Builder historyBuilder = - new NetworkStatsHistory.Builder(mBucketDurationMillis, historyEntries.size()); + new NetworkStatsHistory.Builder(mBucketDurationMillis, size); + + // TODO: this simply appends the entries to any entries that were already present in + // the builder, which requires the caller to pass in entries in order. We might be + // able to do better with something like recordHistory. + if (existing != null) { + for (Entry entry : existing.getEntries()) { + historyBuilder.addEntry(entry); + } + } + for (Entry entry : historyEntries) { historyBuilder.addEntry(entry); } diff --git a/framework-t/src/android/net/NetworkStatsHistory.java b/framework-t/src/android/net/NetworkStatsHistory.java index 60dad905b2..0ff9d96c29 100644 --- a/framework-t/src/android/net/NetworkStatsHistory.java +++ b/framework-t/src/android/net/NetworkStatsHistory.java @@ -1136,14 +1136,44 @@ public final class NetworkStatsHistory implements Parcelable { mOperations = new ArrayList<>(initialCapacity); } + private void addToElement(List list, int pos, long value) { + list.set(pos, list.get(pos) + value); + } + /** * Add an {@link Entry} into the {@link NetworkStatsHistory} instance. * - * @param entry The target {@link Entry} object. + * @param entry The target {@link Entry} object. The entry timestamp must be greater than + * that of any previously-added entry. * @return The builder object. */ @NonNull public Builder addEntry(@NonNull Entry entry) { + final int lastBucket = mBucketStart.size() - 1; + final long lastBucketStart = (lastBucket != -1) ? mBucketStart.get(lastBucket) : 0; + + // If last bucket has the same timestamp, modify it instead of adding another bucket. + // This allows callers to pass in the same bucket twice (e.g., to accumulate + // data over time), but still requires that entries must be sorted. + // The importer will do this in case a rotated file has the same timestamp as + // the previous file. + if (lastBucket != -1 && entry.bucketStart == lastBucketStart) { + addToElement(mActiveTime, lastBucket, entry.activeTime); + addToElement(mRxBytes, lastBucket, entry.rxBytes); + addToElement(mRxPackets, lastBucket, entry.rxPackets); + addToElement(mTxBytes, lastBucket, entry.txBytes); + addToElement(mTxPackets, lastBucket, entry.txPackets); + addToElement(mOperations, lastBucket, entry.operations); + return this; + } + + // Inserting in the middle is prohibited for performance reasons. + if (entry.bucketStart <= lastBucketStart) { + throw new IllegalArgumentException("new bucket start " + entry.bucketStart + + " must be greater than last bucket start " + lastBucketStart); + } + + // Common case: add entries at the end of the list. mBucketStart.add(entry.bucketStart); mActiveTime.add(entry.activeTime); mRxBytes.add(entry.rxBytes); diff --git a/tests/common/java/android/net/netstats/NetworkStatsHistoryTest.kt b/tests/common/java/android/net/netstats/NetworkStatsHistoryTest.kt index c2654c54a5..f8e041ab92 100644 --- a/tests/common/java/android/net/netstats/NetworkStatsHistoryTest.kt +++ b/tests/common/java/android/net/netstats/NetworkStatsHistoryTest.kt @@ -27,6 +27,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.JUnit4 import kotlin.test.assertEquals +import kotlin.test.assertFailsWith @ConnectivityModuleTest @RunWith(JUnit4::class) @@ -51,12 +52,22 @@ class NetworkStatsHistoryTest { .build() statsSingle.assertEntriesEqual(entry1) assertEquals(DateUtils.HOUR_IN_MILLIS, statsSingle.bucketDuration) + + // Verify the builder throws if the timestamp of added entry is not greater than + // that of any previously-added entry. + assertFailsWith(IllegalArgumentException::class) { + NetworkStatsHistory + .Builder(DateUtils.SECOND_IN_MILLIS, /* initialCapacity */ 0) + .addEntry(entry1).addEntry(entry2).addEntry(entry3) + .build() + } + val statsMultiple = NetworkStatsHistory .Builder(DateUtils.SECOND_IN_MILLIS, /* initialCapacity */ 0) - .addEntry(entry1).addEntry(entry2).addEntry(entry3) + .addEntry(entry3).addEntry(entry1).addEntry(entry2) .build() assertEquals(DateUtils.SECOND_IN_MILLIS, statsMultiple.bucketDuration) - statsMultiple.assertEntriesEqual(entry1, entry2, entry3) + statsMultiple.assertEntriesEqual(entry3, entry1, entry2) } fun NetworkStatsHistory.assertEntriesEqual(vararg entries: NetworkStatsHistory.Entry) { diff --git a/tests/unit/java/android/net/netstats/NetworkStatsDataMigrationUtilsTest.kt b/tests/unit/java/android/net/netstats/NetworkStatsDataMigrationUtilsTest.kt index 743d39e6cd..aa5a246aba 100644 --- a/tests/unit/java/android/net/netstats/NetworkStatsDataMigrationUtilsTest.kt +++ b/tests/unit/java/android/net/netstats/NetworkStatsDataMigrationUtilsTest.kt @@ -61,14 +61,6 @@ class NetworkStatsDataMigrationUtilsTest { assertValues(builder.build(), 55, 1814302L, 21050L, 31001636L, 26152L) } - @Test - fun testMaybeReadLegacyUid() { - val builder = NetworkStatsCollection.Builder(BUCKET_DURATION_MS) - NetworkStatsDataMigrationUtils.readLegacyUid(builder, - getInputStreamForResource(R.raw.netstats_uid_v4), false /* taggedData */) - assertValues(builder.build(), 223, 106245210L, 710722L, 1130647496L, 1103989L) - } - private fun assertValues( collection: NetworkStatsCollection, expectedSize: Int, From 1190ea3eb7bc39b757034bf2fa5ebec32b72a2e3 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Thu, 19 May 2022 15:09:42 +0800 Subject: [PATCH 5/5] Skip PersistentIntTest on S- device This class is only used in the service runs on T+ devices. Thus run this test on S- device does not make senses. Bug: 230289468 Test: atest ConnectivityCoverageTests:PersistentIntTest on S device Change-Id: I97a29df17ef9ee9ab373655f045cdb5c820e06ef Merged-In: I97a29df17ef9ee9ab373655f045cdb5c820e06ef (pure cherry-picked from ag/18486287) --- .../unit/java/com/android/server/net/PersistentIntTest.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/unit/java/com/android/server/net/PersistentIntTest.kt b/tests/unit/java/com/android/server/net/PersistentIntTest.kt index 9509d38549..92683525ed 100644 --- a/tests/unit/java/com/android/server/net/PersistentIntTest.kt +++ b/tests/unit/java/com/android/server/net/PersistentIntTest.kt @@ -17,10 +17,14 @@ package com.android.server.net import android.util.SystemConfigFileCommitEventLogger +import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo +import com.android.testutils.DevSdkIgnoreRunner +import com.android.testutils.SC_V2 import com.android.testutils.assertThrows import org.junit.After import org.junit.Before import org.junit.Test +import org.junit.runner.RunWith import java.io.File import java.io.IOException import java.nio.file.Files @@ -32,6 +36,8 @@ import java.nio.file.attribute.PosixFilePermission.OWNER_WRITE import java.util.Random import kotlin.test.assertEquals +@RunWith(DevSdkIgnoreRunner::class) +@IgnoreUpTo(SC_V2) class PersistentIntTest { val tempFilesCreated = mutableSetOf() lateinit var tempDir: Path @@ -124,4 +130,4 @@ class PersistentIntTest { tempFilesCreated.add(it) } } -} \ No newline at end of file +}