Merge "Fix statsd NPE on setPullAtomCallback" into rvc-dev
diff --git a/apex/framework/Android.bp b/apex/framework/Android.bp
index 6f29141..9f5d933 100644
--- a/apex/framework/Android.bp
+++ b/apex/framework/Android.bp
@@ -84,6 +84,7 @@
"framework-annotations-lib",
],
sdk_version: "system_current",
+ dist: { dest: "framework-statsd.txt" },
}
droidstubs {
@@ -92,6 +93,15 @@
"framework-module-stubs-defaults-publicapi",
"framework-statsd-stubs-srcs-defaults",
],
+ check_api: {
+ last_released: {
+ api_file: ":framework-statsd.api.public.latest",
+ removed_api_file: ":framework-statsd-removed.api.public.latest",
+ },
+ api_lint: {
+ new_since: ":framework-statsd.api.public.latest",
+ },
+ },
}
droidstubs {
@@ -100,7 +110,15 @@
"framework-module-stubs-defaults-systemapi",
"framework-statsd-stubs-srcs-defaults",
],
-
+ check_api: {
+ last_released: {
+ api_file: ":framework-statsd.api.system.latest",
+ removed_api_file: ":framework-statsd-removed.api.system.latest",
+ },
+ api_lint: {
+ new_since: ":framework-statsd.api.system.latest",
+ },
+ },
}
droidstubs {
@@ -109,6 +127,15 @@
"framework-module-api-defaults-module_libs_api",
"framework-statsd-stubs-srcs-defaults",
],
+ check_api: {
+ last_released: {
+ api_file: ":framework-statsd.api.module-lib.latest",
+ removed_api_file: ":framework-statsd-removed.api.module-lib.latest",
+ },
+ api_lint: {
+ new_since: ":framework-statsd.api.module-lib.latest",
+ },
+ },
}
droidstubs {
@@ -127,6 +154,7 @@
"//frameworks/base", // Framework
"//frameworks/base/apex/statsd", // statsd apex
],
+ dist: { dest: "framework-statsd.jar" },
}
java_library {
@@ -137,6 +165,7 @@
"//frameworks/base", // Framework
"//frameworks/base/apex/statsd", // statsd apex
],
+ dist: { dest: "framework-statsd.jar" },
}
java_library {
@@ -149,6 +178,7 @@
"//frameworks/opt/net/wifi/service", // wifi service
"//packages/providers/MediaProvider", // MediaProvider apk
],
+ dist: { dest: "framework-statsd.jar" },
}
android_test {
diff --git a/apex/service/java/com/android/server/stats/StatsCompanionService.java b/apex/service/java/com/android/server/stats/StatsCompanionService.java
index 93e6c10..5cf5e0b 100644
--- a/apex/service/java/com/android/server/stats/StatsCompanionService.java
+++ b/apex/service/java/com/android/server/stats/StatsCompanionService.java
@@ -54,11 +54,11 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.PrintWriter;
-import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
/**
* Helper service for statsd (the native stats management service in cmds/statsd/).
@@ -112,17 +112,8 @@
private final HashMap<Long, String> mDeletedFiles = new HashMap<>();
private final CompanionHandler mHandler;
- // Flag that is set when PHASE_BOOT_COMPLETED is triggered in the StatsCompanion lifecycle. This
- // and the flag mSentBootComplete below is used for synchronization to ensure that the boot
- // complete signal is only ever sent once to statsd. Two signals are needed because
- // #sayHiToStatsd can be called from both statsd and #onBootPhase
- // PHASE_THIRD_PARTY_APPS_CAN_START.
- @GuardedBy("sStatsdLock")
- private boolean mBootCompleted = false;
- // Flag that is set when IStatsd#bootCompleted is called. This flag ensures that boot complete
- // signal is only ever sent once.
- @GuardedBy("sStatsdLock")
- private boolean mSentBootComplete = false;
+ // Flag that is set when PHASE_BOOT_COMPLETED is triggered in the StatsCompanion lifecycle.
+ private AtomicBoolean mBootCompleted = new AtomicBoolean(false);
public StatsCompanionService(Context context) {
super();
@@ -607,27 +598,35 @@
// Statsd related code
/**
- * Fetches the statsd IBinder service. This is a blocking call.
+ * Fetches the statsd IBinder service. This is a blocking call that always refetches statsd
+ * instead of returning the cached sStatsd.
* Note: This should only be called from {@link #sayHiToStatsd()}. All other clients should use
* the cached sStatsd via {@link #getStatsdNonblocking()}.
*/
- private IStatsd fetchStatsdService(StatsdDeathRecipient deathRecipient) {
- synchronized (sStatsdLock) {
- if (sStatsd == null) {
- sStatsd = IStatsd.Stub.asInterface(StatsFrameworkInitializer
- .getStatsServiceManager()
- .getStatsdServiceRegisterer()
- .get());
- if (sStatsd != null) {
- try {
- sStatsd.asBinder().linkToDeath(deathRecipient, /* flags */ 0);
- } catch (RemoteException e) {
- Log.e(TAG, "linkToDeath(StatsdDeathRecipient) failed");
- statsdNotReadyLocked();
- }
+ private IStatsd fetchStatsdServiceLocked() {
+ sStatsd = IStatsd.Stub.asInterface(StatsFrameworkInitializer
+ .getStatsServiceManager()
+ .getStatsdServiceRegisterer()
+ .get());
+ return sStatsd;
+ }
+
+ private void registerStatsdDeathRecipient(IStatsd statsd, List<BroadcastReceiver> receivers) {
+ StatsdDeathRecipient deathRecipient = new StatsdDeathRecipient(statsd, receivers);
+
+ try {
+ statsd.asBinder().linkToDeath(deathRecipient, /*flags=*/0);
+ } catch (RemoteException e) {
+ Log.e(TAG, "linkToDeath (StatsdDeathRecipient) failed");
+ // Statsd has already died. Unregister receivers ourselves.
+ for (BroadcastReceiver receiver : receivers) {
+ mContext.unregisterReceiver(receiver);
+ }
+ synchronized (sStatsdLock) {
+ if (statsd == sStatsd) {
+ statsdNotReadyLocked();
}
}
- return sStatsd;
}
}
@@ -648,22 +647,23 @@
* statsd.
*/
private void sayHiToStatsd() {
- if (getStatsdNonblocking() != null) {
- Log.e(TAG, "Trying to fetch statsd, but it was already fetched",
- new IllegalStateException(
- "sStatsd is not null when being fetched"));
- return;
+ IStatsd statsd;
+ synchronized (sStatsdLock) {
+ if (sStatsd != null && sStatsd.asBinder().isBinderAlive()) {
+ Log.e(TAG, "statsd has already been fetched before",
+ new IllegalStateException("IStatsd object should be null or dead"));
+ return;
+ }
+ statsd = fetchStatsdServiceLocked();
}
- StatsdDeathRecipient deathRecipient = new StatsdDeathRecipient();
- IStatsd statsd = fetchStatsdService(deathRecipient);
+
if (statsd == null) {
- Log.i(TAG,
- "Could not yet find statsd to tell it that StatsCompanion is "
- + "alive.");
+ Log.i(TAG, "Could not yet find statsd to tell it that StatsCompanion is alive.");
return;
}
- mStatsManagerService.statsdReady(statsd);
+
if (DEBUG) Log.d(TAG, "Saying hi to statsd");
+ mStatsManagerService.statsdReady(statsd);
try {
statsd.statsCompanionReady();
@@ -682,8 +682,7 @@
mContext.registerReceiverForAllUsers(appUpdateReceiver, filter, null, null);
// Setup receiver for user initialize (which happens once for a new user)
- // and
- // if a user is removed.
+ // and if a user is removed.
filter = new IntentFilter(Intent.ACTION_USER_INITIALIZE);
filter.addAction(Intent.ACTION_USER_REMOVED);
mContext.registerReceiverForAllUsers(userUpdateReceiver, filter, null, null);
@@ -691,27 +690,20 @@
// Setup receiver for device reboots or shutdowns.
filter = new IntentFilter(Intent.ACTION_REBOOT);
filter.addAction(Intent.ACTION_SHUTDOWN);
- mContext.registerReceiverForAllUsers(
- shutdownEventReceiver, filter, null, null);
+ mContext.registerReceiverForAllUsers(shutdownEventReceiver, filter, null, null);
- // Only add the receivers if the registration is successful.
- deathRecipient.addRegisteredBroadcastReceivers(
- List.of(appUpdateReceiver, userUpdateReceiver, shutdownEventReceiver));
+ // Register death recipient.
+ List<BroadcastReceiver> broadcastReceivers =
+ List.of(appUpdateReceiver, userUpdateReceiver, shutdownEventReceiver);
+ registerStatsdDeathRecipient(statsd, broadcastReceivers);
- // Used so we can call statsd.bootComplete() outside of the lock.
- boolean shouldSendBootComplete = false;
- synchronized (sStatsdLock) {
- if (mBootCompleted && !mSentBootComplete) {
- mSentBootComplete = true;
- shouldSendBootComplete = true;
- }
- }
- if (shouldSendBootComplete) {
+ // Tell statsd that boot has completed. The signal may have already been sent, but since
+ // the signal-receiving function is idempotent, that's ok.
+ if (mBootCompleted.get()) {
statsd.bootCompleted();
}
- // Pull the latest state of UID->app name, version mapping when
- // statsd starts.
+ // Pull the latest state of UID->app name, version mapping when statsd starts.
informAllUids(mContext);
Log.i(TAG, "Told statsd that StatsCompanionService is alive.");
@@ -722,18 +714,16 @@
private class StatsdDeathRecipient implements IBinder.DeathRecipient {
- private List<BroadcastReceiver> mReceiversToUnregister;
+ private final IStatsd mStatsd;
+ private final List<BroadcastReceiver> mReceiversToUnregister;
- StatsdDeathRecipient() {
- mReceiversToUnregister = new ArrayList<>();
+ StatsdDeathRecipient(IStatsd statsd, List<BroadcastReceiver> receivers) {
+ mStatsd = statsd;
+ mReceiversToUnregister = receivers;
}
- public void addRegisteredBroadcastReceivers(List<BroadcastReceiver> receivers) {
- synchronized (sStatsdLock) {
- mReceiversToUnregister.addAll(receivers);
- }
- }
-
+ // It is possible for binderDied to be called after a restarted statsd calls statsdReady,
+ // but that's alright because the code does not assume an ordering of the two calls.
@Override
public void binderDied() {
Log.i(TAG, "Statsd is dead - erase all my knowledge, except pullers");
@@ -762,13 +752,19 @@
}
}
}
- // We only unregister in binder death becaseu receivers can only be unregistered
- // once, or an IllegalArgumentException is thrown.
+
+ // Unregister receivers on death because receivers can only be unregistered once.
+ // Otherwise, an IllegalArgumentException is thrown.
for (BroadcastReceiver receiver: mReceiversToUnregister) {
mContext.unregisterReceiver(receiver);
}
- statsdNotReadyLocked();
- mSentBootComplete = false;
+
+ // It's possible for statsd to have restarted and called statsdReady, causing a new
+ // sStatsd binder object to be fetched, before the binderDied callback runs. Only
+ // call #statsdNotReadyLocked if that hasn't happened yet.
+ if (mStatsd == sStatsd) {
+ statsdNotReadyLocked();
+ }
}
}
}
@@ -779,19 +775,12 @@
}
void bootCompleted() {
+ mBootCompleted.set(true);
IStatsd statsd = getStatsdNonblocking();
- synchronized (sStatsdLock) {
- mBootCompleted = true;
- if (mSentBootComplete) {
- // do not send a boot complete a second time.
- return;
- }
- if (statsd == null) {
- // Statsd is not yet ready.
- // Delay the boot completed ping to {@link #sayHiToStatsd()}
- return;
- }
- mSentBootComplete = true;
+ if (statsd == null) {
+ // Statsd is not yet ready.
+ // Delay the boot completed ping to {@link #sayHiToStatsd()}
+ return;
}
try {
statsd.bootCompleted();
@@ -808,8 +797,7 @@
}
synchronized (sStatsdLock) {
- writer.println(
- "Number of configuration files deleted: " + mDeletedFiles.size());
+ writer.println("Number of configuration files deleted: " + mDeletedFiles.size());
if (mDeletedFiles.size() > 0) {
writer.println(" timestamp, deleted file name");
}
@@ -817,8 +805,7 @@
SystemClock.currentThreadTimeMillis() - SystemClock.elapsedRealtime();
for (Long elapsedMillis : mDeletedFiles.keySet()) {
long deletionMillis = lastBootMillis + elapsedMillis;
- writer.println(
- " " + deletionMillis + ", " + mDeletedFiles.get(elapsedMillis));
+ writer.println(" " + deletionMillis + ", " + mDeletedFiles.get(elapsedMillis));
}
}
}