[RESTRICT AUTOMERGE]: Fix a race condition in the crash reporter
problems fixed:
* logcat wasn't cleared before checking for crashes
* unrelated crashes could be reported as part of the current test
* The test could start before the reporter was started
* matching a crash before the test start caused a nullpointerexception
Bug: 162378163
Test: adb shell kill -11 mediaextractor # during run for StagefrightTests
Change-Id: Ie31d1f47c3485808a89c4b33addd303914cb36a4
Merged-In: Ie31d1f47c3485808a89c4b33addd303914cb36a4
diff --git a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/targetprep/CrashReporter.java b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/targetprep/CrashReporter.java
index c8a7dfe..58133bb 100644
--- a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/targetprep/CrashReporter.java
+++ b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/targetprep/CrashReporter.java
@@ -47,6 +47,9 @@
/** Uploads the current buffer of Crashes to the phone under the current test name. */
private static void upload(ITestDevice device, String testname, List<Crash> crashes) {
+ if (crashes == null) {
+ crashes = new ArrayList<>();
+ }
try {
if (testname == null) {
CLog.logAndDisplay(LogLevel.ERROR, "Attempted upload with no test name");
@@ -79,6 +82,13 @@
@Override
public void setUp(ITestDevice device, IBuildInfo buildInfo) {
try {
+ device.executeShellCommand("logcat -c");
+ } catch (DeviceNotAvailableException e) {
+ CLog.logAndDisplay(LogLevel.ERROR, "CrashReporterThread failed to clear logcat");
+ CLog.logAndDisplay(LogLevel.ERROR, e.getMessage());
+ }
+
+ try {
device.executeShellCommand("rm -rf " + CrashUtils.DEVICE_PATH);
device.executeShellCommand("mkdir " + CrashUtils.DEVICE_PATH);
} catch (DeviceNotAvailableException e) {
@@ -88,14 +98,23 @@
CLog.logAndDisplay(LogLevel.ERROR, e.getMessage());
return;
}
+
+ CrashReporterReceiver receiver = new CrashReporterReceiver(device);
mBackgroundThread =
new BackgroundDeviceAction(
- "logcat",
- "CrashReporter logcat thread",
- device,
- new CrashReporterReceiver(device),
- 0);
+ "logcat", "CrashReporter logcat thread", device, receiver, 0);
mBackgroundThread.start();
+
+ try {
+ // If the test starts before the reporter receiver can read the test start message then
+ // the crash could only be read halfway. We wait until the receiver starts getting
+ // messages.
+ synchronized (receiver) {
+ receiver.wait(10_000); // wait for 10 seconds max
+ }
+ } catch (InterruptedException e) {
+ // continue as normal
+ }
}
/** {@inheritDoc} */
@@ -133,6 +152,11 @@
mCrashes = new ArrayList<Crash>();
mLogcatChunk.setLength(0);
} else if (CrashUtils.sEndofCrashPattern.matcher(line).find()) {
+ if (mCrashes == null) {
+ // In case the crash happens before any test is started, it's not related to the
+ // current test and shouldn't be reported.
+ return;
+ }
mCrashes = CrashUtils.getAllCrashes(mLogcatChunk.toString());
mLogcatChunk.setLength(0);
} else if (CrashUtils.sUploadRequestPattern.matcher(line).find()) {
@@ -142,6 +166,7 @@
@Override
public void processNewLines(String[] lines) {
+ this.notifyAll(); // alert the main thread that we are active.
if (!isCancelled()) {
for (String line : lines) {
processLogLine(line);
