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

Commit df39d6e4 authored by Yu Shan's avatar Yu Shan
Browse files

Fix a race condition in remote access HAL. 

We should not delete ClientContext after we TryCancel it since the
reader in the main task loop might still access it, e.g., while
calling reader->Finish. We must wait under reader->Finish is
returned, then to delete the ClientContext.

This CL also updates README.md to incorporate soong namespace change.

Test: manually test on sdk_car_x86_64-userdebug
Run TestWakeupClientServer
Run --set-ap-state 1 0 and then --set-ap-state 0 0 multiple times.
Bug: 261234399

Change-Id: I3b0c632546c218c4ced43af95a585cd41e0da036
parent 1532afd3

automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp

+4 −1
Original line number Diff line number Diff line
@@ -111,7 +111,9 @@ void RemoteAccessService::maybeStopTaskLoop() { 
        // Try to stop the reading stream.

        if (mGetRemoteTasksContext) {

            mGetRemoteTasksContext->TryCancel();

            mGetRemoteTasksContext.reset();

            // Don't reset mGetRemoteTaskContext here since the read stream might still be affective

            // and might still be using it. This will cause reader->Read to return false and

            // mGetRemoteTasksContext will be cleared after reader->Finish() is called.

        }

        mTaskWaitStopped = true;

        mCv.notify_all();
@@ -155,6 +157,7 @@ void RemoteAccessService::runTaskLoop() { 
            }

        }

        Status status = reader->Finish();

        mGetRemoteTasksContext.reset();



        ALOGE("GetRemoteTasks stream breaks, code: %d, message: %s, sleeping for 10s and retry",

              status.error_code(), status.error_message().c_str());

automotive/remoteaccess/test_grpc_server/README.md

+20 −2
Original line number Diff line number Diff line
@@ -75,11 +75,18 @@ following behavior: 


* Under android root: `source build/envsetup.sh`



* Add

  ```

  PRODUCT_SOONG_NAMESPACES += hardware/interfaces/automotive/remoteaccess/test_grpc_server/lib`

  ```



  to `device/generic/car/common/car.mk`.



* `lunch sdk_car_x86_64-userdebug`



* `make -j TestWakeupClientServer`



* `make -j ApPowerControlLib`

* `make -j ApPOwerControlLib`



## How to push the test wakeup client to a TCU which runs Android.
@@ -99,7 +106,7 @@ following behavior: 


* `adb push vendor/bin/TestWakeupClientServer /vendor/bin`



* `adb push vendor/lib/ApPowerControlLib.so /vendor/lib`

* `adb push vendor/lib64/ApPowerControlLib.so /vendor/lib64`



* `adb shell`
@@ -116,6 +123,13 @@ interface. 


* Under android root, `source build/envsetup.sh`



* Add

  ```

  PRODUCT_SOONG_NAMESPACES += hardware/interfaces/automotive/remoteaccess/test_grpc_server/lib`

  ```



  to `device/generic/car/common/car.mk`.



* `lunch sdk_car_x86_64-userdebug`



* `m -j`
@@ -150,8 +164,12 @@ interface. 


* `make -j TestWakeupClientServer`



* `make -j ApPOwerControlLib`



* `adb push $ANDROID_PRODUCT_OUT/vendor/bin/TestWakeupClientServer /vendor/bin`



* `adb push $ANDROID_PRODUCT_OUT/vendor/lib64/ApPowerControlLib.so /vendor/lib64`



* `adb shell`



* `emulator_car_x86_64:/ # su`