diff options
3 files changed, 43 insertions, 2 deletions
diff --git a/core/java/com/android/internal/protolog/PerfettoProtoLogImpl.java b/core/java/com/android/internal/protolog/PerfettoProtoLogImpl.java index 4d0cd27fb037..f3dc896a765e 100644 --- a/core/java/com/android/internal/protolog/PerfettoProtoLogImpl.java +++ b/core/java/com/android/internal/protolog/PerfettoProtoLogImpl.java @@ -56,6 +56,7 @@ import android.tracing.perfetto.InitArguments; import android.tracing.perfetto.Producer; import android.tracing.perfetto.TracingContext; import android.util.ArrayMap; +import android.util.ArraySet; import android.util.Log; import android.util.LongArray; import android.util.Slog; @@ -351,6 +352,10 @@ public class PerfettoProtoLogImpl extends IProtoLogClient.Stub implements IProto } private void registerGroupsLocally(@NonNull IProtoLogGroup[] protoLogGroups) { + // Verify we don't have id collisions, if we do we want to know as soon as possible and + // we might want to manually specify an id for the group with a collision + verifyNoCollisionsOrDuplicates(protoLogGroups); + final var groupsLoggingToLogcat = new ArrayList<String>(); for (IProtoLogGroup protoLogGroup : protoLogGroups) { mLogGroups.put(protoLogGroup.name(), protoLogGroup); @@ -369,6 +374,19 @@ public class PerfettoProtoLogImpl extends IProtoLogClient.Stub implements IProto } } + private void verifyNoCollisionsOrDuplicates(@NonNull IProtoLogGroup[] protoLogGroups) { + final var groupId = new ArraySet<Integer>(); + + for (IProtoLogGroup protoLogGroup : protoLogGroups) { + if (groupId.contains(protoLogGroup.getId())) { + throw new RuntimeException( + "Group with same id (" + protoLogGroup.getId() + ") registered twice. " + + "Potential duplicate or hash id collision."); + } + groupId.add(protoLogGroup.getId()); + } + } + /** * Responds to a shell command. */ diff --git a/core/java/com/android/internal/protolog/ProtoLog.java b/core/java/com/android/internal/protolog/ProtoLog.java index adf03fe5f775..60213b1830c6 100644 --- a/core/java/com/android/internal/protolog/ProtoLog.java +++ b/core/java/com/android/internal/protolog/ProtoLog.java @@ -22,8 +22,8 @@ import com.android.internal.protolog.common.IProtoLog; import com.android.internal.protolog.common.IProtoLogGroup; import com.android.internal.protolog.common.LogLevel; -import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; /** * ProtoLog API - exposes static logging methods. Usage of this API is similar @@ -73,7 +73,7 @@ public class ProtoLog { if (sProtoLogInstance != null) { // The ProtoLog instance has already been initialized in this process final var alreadyRegisteredGroups = sProtoLogInstance.getRegisteredGroups(); - final var allGroups = new ArrayList<>(alreadyRegisteredGroups); + final var allGroups = new HashSet<>(alreadyRegisteredGroups); allGroups.addAll(Arrays.stream(groups).toList()); groups = allGroups.toArray(new IProtoLogGroup[0]); } diff --git a/tests/Tracing/src/com/android/internal/protolog/ProtoLogTest.java b/tests/Tracing/src/com/android/internal/protolog/ProtoLogTest.java index 9d56a92fad52..8ecddaa76216 100644 --- a/tests/Tracing/src/com/android/internal/protolog/ProtoLogTest.java +++ b/tests/Tracing/src/com/android/internal/protolog/ProtoLogTest.java @@ -16,6 +16,8 @@ package com.android.internal.protolog; +import static org.junit.Assert.assertThrows; + import android.platform.test.annotations.Presubmit; import com.android.internal.protolog.common.IProtoLogGroup; @@ -44,8 +46,29 @@ public class ProtoLogTest { .containsExactly(TEST_GROUP_1, TEST_GROUP_2); } + @Test + public void throwOnRegisteringDuplicateGroup() { + final var assertion = assertThrows(RuntimeException.class, + () -> ProtoLog.init(TEST_GROUP_1, TEST_GROUP_1, TEST_GROUP_2)); + + Truth.assertThat(assertion).hasMessageThat().contains("" + TEST_GROUP_1.getId()); + Truth.assertThat(assertion).hasMessageThat().contains("duplicate"); + } + + @Test + public void throwOnRegisteringGroupsWithIdCollisions() { + final var assertion = assertThrows(RuntimeException.class, + () -> ProtoLog.init(TEST_GROUP_1, TEST_GROUP_WITH_COLLISION, TEST_GROUP_2)); + + Truth.assertThat(assertion).hasMessageThat() + .contains("" + TEST_GROUP_WITH_COLLISION.getId()); + Truth.assertThat(assertion).hasMessageThat().contains("collision"); + } + private static final IProtoLogGroup TEST_GROUP_1 = new ProtoLogGroup("TEST_TAG_1", 1); private static final IProtoLogGroup TEST_GROUP_2 = new ProtoLogGroup("TEST_TAG_2", 2); + private static final IProtoLogGroup TEST_GROUP_WITH_COLLISION = + new ProtoLogGroup("TEST_TAG_WITH_COLLISION", 1); private static class ProtoLogGroup implements IProtoLogGroup { private final boolean mEnabled; |