From 3da83556dc8db0d703db8e2d7227c11c0b4c7842 Mon Sep 17 00:00:00 2001 From: Howard Date: Thu, 17 Nov 2022 10:58:18 -0800 Subject: Handle NPE for query in ConnectivityPublisher. NetworkStatsService may throw NPE when getUidComplete, we need to handle such cases. Bug: 258519877 Test: atest CarServiceUnitTest:ConnectivityPublisherTest Change-Id: Ie55d1925b903dd5348c714214cc44b7b3f6cb27f Merged-In: Ie55d1925b903dd5348c714214cc44b7b3f6cb27f (cherry picked from commit cbedef222ae7207c12ade9d0142b84f5417d0864) --- .../telemetry/publisher/ConnectivityPublisher.java | 36 ++++++++++++++++------ .../publisher/ConnectivityPublisherTest.java | 27 ++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java b/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java index f161cd5d42..154932e22f 100644 --- a/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java +++ b/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java @@ -173,7 +173,11 @@ public class ConnectivityPublisher extends AbstractPublisher { } try { for (QueryParam param : mSubscribers.keySet()) { - mTransportPreviousNetstats.put(param, getSummaryForAllUid(param)); + RefinedStats summary = getSummaryForAllUid(param); + if (summary == null) { + continue; + } + mTransportPreviousNetstats.put(param, summary); } } catch (RemoteException e) { // Can't do much if the NetworkStatsService is not available. Next netstats pull @@ -233,13 +237,17 @@ public class ConnectivityPublisher extends AbstractPublisher { RefinedStats current; try { current = getSummaryForAllUid(param); - } catch (RemoteException | NullPointerException e) { + } catch (RemoteException e) { // If the NetworkStatsService is not available, it retries in the next pull. Slogf.w(CarLog.TAG_TELEMETRY, e); mTraceLog.traceEnd(); return null; } + if (current == null) { + mTraceLog.traceEnd(); + return null; + } // By subtracting, it calculates network usage since the last pull. RefinedStats diff = RefinedStats.subtract(current, previous); @@ -260,7 +268,7 @@ public class ConnectivityPublisher extends AbstractPublisher { * *

TODO(b/218529196): run this method on a separate thread for better performance. */ - @NonNull + @Nullable private RefinedStats getSummaryForAllUid(@NonNull QueryParam param) throws RemoteException { if (DEBUG) { Slogf.d(CarLog.TAG_TELEMETRY, "getSummaryForAllUid " + param); @@ -273,12 +281,22 @@ public class ConnectivityPublisher extends AbstractPublisher { - elapsedMillisSinceBoot - NETSTATS_UID_DEFAULT_BUCKET_DURATION_MILLIS; - NetworkStatsWrapper nonTaggedStats = - mNetworkStatsManager.querySummary( - param.buildNetworkTemplate(), startMillis, currentTimeInMillis); - NetworkStatsWrapper taggedStats = - mNetworkStatsManager.queryTaggedSummary( - param.buildNetworkTemplate(), startMillis, currentTimeInMillis); + NetworkStatsWrapper nonTaggedStats; + NetworkStatsWrapper taggedStats; + // querySummary and queryTaggedSummary may throw NPE propagated from NetworkStatsService + // when its NetworkStatsRecorder failed to initialize and + // NetworkStatsRecorder.getOrLoadCompleteLocked() is called. + try { + nonTaggedStats = + mNetworkStatsManager.querySummary( + param.buildNetworkTemplate(), startMillis, currentTimeInMillis); + taggedStats = + mNetworkStatsManager.queryTaggedSummary( + param.buildNetworkTemplate(), startMillis, currentTimeInMillis); + } catch (NullPointerException e) { + Slogf.w(CarLog.TAG_TELEMETRY, e); + return null; + } RefinedStats result = new RefinedStats(startMillis, currentTimeInMillis); result.addNetworkStats(nonTaggedStats); diff --git a/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java b/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java index be142d084f..96bb817c0a 100644 --- a/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java +++ b/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java @@ -528,6 +528,21 @@ public class ConnectivityPublisherTest { .asList().containsExactly(1000L); } + @Test + public void testWhenQueryThrowsNullPointerExceptionIsCaught() { + mFakeManager.setSimulateFailedQuery(true); + mSessionControllerCallbackArgumentCaptor.getValue().onSessionStateChanged( + SESSION_ANNOTATION_BEGIN_1); + + // querySummary gets called for each QueryParam combination, but throws + // NullPointerException each time, which is caught + assertThat(mFakeManager.getMethodCallCount("querySummary")) + .isEqualTo(BASELINE_PULL_COUNT); + // queryTaggedSummary not reached because of previous NullPointerException in querySummary + assertThat(mFakeManager.getMethodCallCount("queryTaggedSummary")) + .isEqualTo(0); + } + private static class FakeDataSubscriber extends DataSubscriber { private final ArrayList mPushedData = new ArrayList<>(); @@ -570,6 +585,8 @@ public class ConnectivityPublisherTest { private final ArrayList mBuckets = new ArrayList<>(); private final HashMap mMethodCallCount = new HashMap<>(); + private boolean mSimulateFailedQuery = false; + private FakeNetworkStatsManager() { super(/* networkStatsManager= */ null); } @@ -624,10 +641,17 @@ public class ConnectivityPublisherTest { return mMethodCallCount.getOrDefault(name, 0); } + public void setSimulateFailedQuery(boolean simulateFailedQuery) { + mSimulateFailedQuery = simulateFailedQuery; + } + @Override @NonNull public NetworkStatsWrapper querySummary(NetworkTemplate template, long start, long end) { increaseMethodCall("querySummary", 1); + if (mSimulateFailedQuery) { + throw new NullPointerException(); + } return commonQuerySummary(false, template, start, end); } @@ -636,6 +660,9 @@ public class ConnectivityPublisherTest { public NetworkStatsWrapper queryTaggedSummary( NetworkTemplate template, long start, long end) { increaseMethodCall("queryTaggedSummary", 1); + if (mSimulateFailedQuery) { + throw new NullPointerException(); + } return commonQuerySummary(true, template, start, end); } -- cgit v1.2.3