Make dumpstate listener callbacks synchronous
Dumpstate binder service is oneshot and needs to exit in error
conditions, often right after calling onError. This can make the event
handling on client complex since there could be a race between death
recipient and onError. To make things simpler make onError synchronous.
To keep it consistent make the other callbacks synchronous as well.
Also add a new error code to signal another bugreport is running.
BUG: 123571915
Test: adb shell /data/nativetest64/dumpstate_smoke_test/dumpstate_smoke_test --gtest_filter=DumpstateBinderTest.*
Change-Id: I33f0b3a080ba493dba3521439daa6a46354a8470
(cherry picked from commit e64c3aa814b8cbd62907b8026899533a0f24c8f6)
diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp
index b90941ac..768cb4f 100644
--- a/cmds/dumpstate/DumpstateService.cpp
+++ b/cmds/dumpstate/DumpstateService.cpp
@@ -127,7 +127,7 @@
if (ds_ != nullptr) {
MYLOGE("Error! There is already a bugreport in progress. Returning.");
if (listener != nullptr) {
- listener->onError(IDumpstateListener::BUGREPORT_ERROR_RUNTIME_ERROR);
+ listener->onError(IDumpstateListener::BUGREPORT_ERROR_CONCURRENT_BUGREPORTS_FORBIDDEN);
}
return exception(binder::Status::EX_SERVICE_SPECIFIC,
"There is already a bugreport in progress");
diff --git a/cmds/dumpstate/binder/android/os/IDumpstate.aidl b/cmds/dumpstate/binder/android/os/IDumpstate.aidl
index b1005d3..37ff442 100644
--- a/cmds/dumpstate/binder/android/os/IDumpstate.aidl
+++ b/cmds/dumpstate/binder/android/os/IDumpstate.aidl
@@ -36,6 +36,9 @@
IDumpstateToken setListener(@utf8InCpp String name, IDumpstateListener listener,
boolean getSectionDetails);
+ // NOTE: If you add to or change these modes, please also change the corresponding enums
+ // in system server, in BugreportParams.java.
+
// These modes encapsulate a set of run time options for generating bugreports.
// Takes a bugreport without user interference.
const int BUGREPORT_MODE_FULL = 0;
diff --git a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
index 907a67c..42858e0 100644
--- a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
+++ b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
@@ -19,6 +19,11 @@
/**
* Listener for dumpstate events.
*
+ * <p>When bugreport creation is complete one of {@code onError} or {@code onFinished} is called.
+ *
+ * <p>These methods are synchronous by design in order to make dumpstate's lifecycle simpler
+ * to handle.
+ *
* {@hide}
*/
interface IDumpstateListener {
@@ -27,7 +32,10 @@
*
* @param progress the progress in [0, 100]
*/
- oneway void onProgress(int progress);
+ void onProgress(int progress);
+
+ // NOTE: If you add to or change these error codes, please also change the corresponding enums
+ // in system server, in BugreportManager.java.
/* Options specified are invalid or incompatible */
const int BUGREPORT_ERROR_INVALID_INPUT = 1;
@@ -41,15 +49,18 @@
/* The request to get user consent timed out */
const int BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT = 4;
+ /* There is currently a bugreport running. The caller should try again later. */
+ const int BUGREPORT_ERROR_CONCURRENT_BUGREPORTS_FORBIDDEN = 5;
+
/**
* Called on an error condition with one of the error codes listed above.
*/
- oneway void onError(int errorCode);
+ void onError(int errorCode);
/**
* Called when taking bugreport finishes successfully.
*/
- oneway void onFinished();
+ void onFinished();
// TODO(b/111441001): Remove old methods when not used anymore.
void onProgressUpdated(int progress);
diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
index b5ad699..854ca76 100644
--- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
@@ -474,7 +474,8 @@
Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener2);
EXPECT_FALSE(status.isOk());
WaitTillExecutionComplete(listener2.get());
- EXPECT_EQ(listener2->getErrorCode(), IDumpstateListener::BUGREPORT_ERROR_RUNTIME_ERROR);
+ EXPECT_EQ(listener2->getErrorCode(),
+ IDumpstateListener::BUGREPORT_ERROR_CONCURRENT_BUGREPORTS_FORBIDDEN);
// Meanwhile the first call works as expected. Service should not die in this case.
WaitTillExecutionComplete(listener1.get());