Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Handle race conditions in SCS when statsd dies" into rvc-dev

Browse Source
This commit is contained in:
Ruchir Rastogi
2020-05-06 22:10:42 +00:00
committed by Android (Google) Code Review
parent c9dad09ad3 e92edbaffc
commit 76994e73a7
2 changed files with 75 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

131
apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java
View File

@@ -54,11 +54,11 @@ import java.io.FileDescriptor;
import java.io.FileOutputStream; import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
import java.io.PrintWriter; import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
/** /**
* Helper service for statsd (the native stats management service in cmds/statsd/). * Helper service for statsd (the native stats management service in cmds/statsd/).
@@ -112,17 +112,8 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
private final HashMap<Long, String> mDeletedFiles = new HashMap<>(); private final HashMap<Long, String> mDeletedFiles = new HashMap<>();
private final CompanionHandler mHandler; private final CompanionHandler mHandler;
// Flag that is set when PHASE_BOOT_COMPLETED is triggered in the StatsCompanion lifecycle. This // Flag that is set when PHASE_BOOT_COMPLETED is triggered in the StatsCompanion lifecycle.
// and the flag mSentBootComplete below is used for synchronization to ensure that the boot private AtomicBoolean mBootCompleted = new AtomicBoolean(false);
// 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;
public StatsCompanionService(Context context) { public StatsCompanionService(Context context) {
super(); super();
@@ -607,28 +598,36 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
// Statsd related code // 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 * Note: This should only be called from {@link #sayHiToStatsd()}. All other clients should use
* the cached sStatsd via {@link #getStatsdNonblocking()}. * the cached sStatsd via {@link #getStatsdNonblocking()}.
*/ */
private IStatsd fetchStatsdService(StatsdDeathRecipient deathRecipient) { private IStatsd fetchStatsdServiceLocked() {
synchronized (sStatsdLock) {
if (sStatsd == null) {
sStatsd = IStatsd.Stub.asInterface(StatsFrameworkInitializer sStatsd = IStatsd.Stub.asInterface(StatsFrameworkInitializer
.getStatsServiceManager() .getStatsServiceManager()
.getStatsdServiceRegisterer() .getStatsdServiceRegisterer()
.get()); .get());
if (sStatsd != null) { return sStatsd;
}
private void registerStatsdDeathRecipient(IStatsd statsd, List<BroadcastReceiver> receivers) {
StatsdDeathRecipient deathRecipient = new StatsdDeathRecipient(statsd, receivers);
try { try {
sStatsd.asBinder().linkToDeath(deathRecipient, /* flags */ 0); statsd.asBinder().linkToDeath(deathRecipient, /*flags=*/0);
} catch (RemoteException e) { } catch (RemoteException e) {
Log.e(TAG, "linkToDeath (StatsdDeathRecipient) failed"); 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(); statsdNotReadyLocked();
} }
} }
} }
return sStatsd;
}
} }
/** /**
@@ -648,22 +647,23 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
* statsd. * statsd.
*/ */
private void sayHiToStatsd() { private void sayHiToStatsd() {
if (getStatsdNonblocking() != null) { IStatsd statsd;
Log.e(TAG, "Trying to fetch statsd, but it was already fetched", synchronized (sStatsdLock) {
new IllegalStateException( if (sStatsd != null && sStatsd.asBinder().isBinderAlive()) {
"sStatsd is not null when being fetched")); Log.e(TAG, "statsd has already been fetched before",
new IllegalStateException("IStatsd object should be null or dead"));
return; return;
} }
StatsdDeathRecipient deathRecipient = new StatsdDeathRecipient(); statsd = fetchStatsdServiceLocked();
IStatsd statsd = fetchStatsdService(deathRecipient); }
if (statsd == null) { if (statsd == null) {
Log.i(TAG, Log.i(TAG, "Could not yet find statsd to tell it that StatsCompanion is alive.");
"Could not yet find statsd to tell it that StatsCompanion is "
+ "alive.");
return; return;
} }
mStatsManagerService.statsdReady(statsd);
if (DEBUG) Log.d(TAG, "Saying hi to statsd"); if (DEBUG) Log.d(TAG, "Saying hi to statsd");
mStatsManagerService.statsdReady(statsd);
try { try {
statsd.statsCompanionReady(); statsd.statsCompanionReady();
@@ -682,8 +682,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
mContext.registerReceiverForAllUsers(appUpdateReceiver, filter, null, null); mContext.registerReceiverForAllUsers(appUpdateReceiver, filter, null, null);
// Setup receiver for user initialize (which happens once for a new user) // Setup receiver for user initialize (which happens once for a new user)
// and // and if a user is removed.
// if a user is removed.
filter = new IntentFilter(Intent.ACTION_USER_INITIALIZE); filter = new IntentFilter(Intent.ACTION_USER_INITIALIZE);
filter.addAction(Intent.ACTION_USER_REMOVED); filter.addAction(Intent.ACTION_USER_REMOVED);
mContext.registerReceiverForAllUsers(userUpdateReceiver, filter, null, null); mContext.registerReceiverForAllUsers(userUpdateReceiver, filter, null, null);
@@ -691,27 +690,20 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
// Setup receiver for device reboots or shutdowns. // Setup receiver for device reboots or shutdowns.
filter = new IntentFilter(Intent.ACTION_REBOOT); filter = new IntentFilter(Intent.ACTION_REBOOT);
filter.addAction(Intent.ACTION_SHUTDOWN); filter.addAction(Intent.ACTION_SHUTDOWN);
mContext.registerReceiverForAllUsers( mContext.registerReceiverForAllUsers(shutdownEventReceiver, filter, null, null);
shutdownEventReceiver, filter, null, null);
// Only add the receivers if the registration is successful. // Register death recipient.
deathRecipient.addRegisteredBroadcastReceivers( List<BroadcastReceiver> broadcastReceivers =
List.of(appUpdateReceiver, userUpdateReceiver, shutdownEventReceiver)); List.of(appUpdateReceiver, userUpdateReceiver, shutdownEventReceiver);
registerStatsdDeathRecipient(statsd, broadcastReceivers);
// Used so we can call statsd.bootComplete() outside of the lock. // Tell statsd that boot has completed. The signal may have already been sent, but since
boolean shouldSendBootComplete = false; // the signal-receiving function is idempotent, that's ok.
synchronized (sStatsdLock) { if (mBootCompleted.get()) {
if (mBootCompleted && !mSentBootComplete) {
mSentBootComplete = true;
shouldSendBootComplete = true;
}
}
if (shouldSendBootComplete) {
statsd.bootCompleted(); statsd.bootCompleted();
} }
// Pull the latest state of UID->app name, version mapping when // Pull the latest state of UID->app name, version mapping when statsd starts.
// statsd starts.
informAllUids(mContext); informAllUids(mContext);
Log.i(TAG, "Told statsd that StatsCompanionService is alive."); Log.i(TAG, "Told statsd that StatsCompanionService is alive.");
@@ -722,18 +714,16 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
private class StatsdDeathRecipient implements IBinder.DeathRecipient { private class StatsdDeathRecipient implements IBinder.DeathRecipient {
private List<BroadcastReceiver> mReceiversToUnregister; private final IStatsd mStatsd;
private final List<BroadcastReceiver> mReceiversToUnregister;
StatsdDeathRecipient() { StatsdDeathRecipient(IStatsd statsd, List<BroadcastReceiver> receivers) {
mReceiversToUnregister = new ArrayList<>(); 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 @Override
public void binderDied() { public void binderDied() {
Log.i(TAG, "Statsd is dead - erase all my knowledge, except pullers"); Log.i(TAG, "Statsd is dead - erase all my knowledge, except pullers");
@@ -762,13 +752,19 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
} }
} }
} }
// 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) { for (BroadcastReceiver receiver: mReceiversToUnregister) {
mContext.unregisterReceiver(receiver); mContext.unregisterReceiver(receiver);
} }
// 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(); statsdNotReadyLocked();
mSentBootComplete = false; }
} }
} }
} }
@@ -779,20 +775,13 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
} }
void bootCompleted() { void bootCompleted() {
mBootCompleted.set(true);
IStatsd statsd = getStatsdNonblocking(); IStatsd statsd = getStatsdNonblocking();
synchronized (sStatsdLock) {
mBootCompleted = true;
if (mSentBootComplete) {
// do not send a boot complete a second time.
return;
}
if (statsd == null) { if (statsd == null) {
// Statsd is not yet ready. // Statsd is not yet ready.
// Delay the boot completed ping to {@link #sayHiToStatsd()} // Delay the boot completed ping to {@link #sayHiToStatsd()}
return; return;
} }
mSentBootComplete = true;
}
try { try {
statsd.bootCompleted(); statsd.bootCompleted();
} catch (RemoteException e) { } catch (RemoteException e) {
@@ -808,8 +797,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
} }
synchronized (sStatsdLock) { synchronized (sStatsdLock) {
writer.println( writer.println("Number of configuration files deleted: " + mDeletedFiles.size());
"Number of configuration files deleted: " + mDeletedFiles.size());
if (mDeletedFiles.size() > 0) { if (mDeletedFiles.size() > 0) {
writer.println(" timestamp, deleted file name"); writer.println(" timestamp, deleted file name");
} }
@@ -817,8 +805,7 @@ public class StatsCompanionService extends IStatsCompanionService.Stub {
SystemClock.currentThreadTimeMillis() - SystemClock.elapsedRealtime(); SystemClock.currentThreadTimeMillis() - SystemClock.elapsedRealtime();
for (Long elapsedMillis : mDeletedFiles.keySet()) { for (Long elapsedMillis : mDeletedFiles.keySet()) {
long deletionMillis = lastBootMillis + elapsedMillis; long deletionMillis = lastBootMillis + elapsedMillis;
writer.println( writer.println(" " + deletionMillis + ", " + mDeletedFiles.get(elapsedMillis));
" " + deletionMillis + ", " + mDeletedFiles.get(elapsedMillis));
} }
} }
} }

2
cmds/statsd/src/StatsService.cpp
View File

@@ -343,9 +343,11 @@ status_t StatsService::handleShellCommand(int in, int out, int err, const char**
if (!utf8Args[0].compare(String8("print-logs"))) { if (!utf8Args[0].compare(String8("print-logs"))) {
return cmd_print_logs(out, utf8Args); return cmd_print_logs(out, utf8Args);
} }
if (!utf8Args[0].compare(String8("send-active-configs"))) { if (!utf8Args[0].compare(String8("send-active-configs"))) {
return cmd_trigger_active_config_broadcast(out, utf8Args); return cmd_trigger_active_config_broadcast(out, utf8Args);
} }
if (!utf8Args[0].compare(String8("data-subscribe"))) { if (!utf8Args[0].compare(String8("data-subscribe"))) {
{ {
std::lock_guard<std::mutex> lock(mShellSubscriberMutex); std::lock_guard<std::mutex> lock(mShellSubscriberMutex);