Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 41d7dac7 authored by Nandana Dutt's avatar Nandana Dutt
Browse files

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)
parent 16d1aeec

cmds/dumpstate/DumpstateService.cpp

+1 −1
Original line number Diff line number Diff line
@@ -127,7 +127,7 @@ binder::Status DumpstateService::startBugreport(int32_t calling_uid, 
    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");

cmds/dumpstate/binder/android/os/IDumpstate.aidl

+3 −0
Original line number Diff line number Diff line
@@ -36,6 +36,9 @@ interface IDumpstate { 
    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;

cmds/dumpstate/binder/android/os/IDumpstateListener.aidl

+14 −3
Original line number Diff line number Diff line
@@ -19,6 +19,11 @@ package android.os; 
/**

  * 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 @@ interface IDumpstateListener { 
     *

     * @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 @@ interface IDumpstateListener { 
    /* 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);

cmds/dumpstate/tests/dumpstate_smoke_test.cpp

+2 −1
Original line number Diff line number Diff line
@@ -474,7 +474,8 @@ TEST_F(DumpstateBinderTest, SimultaneousBugreportsNotAllowed) { 
                                       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());