From 1532afd33c20142d35a9e4d30168f46ec884b1cb Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Wed, 30 Nov 2022 14:41:30 -0800 Subject: [PATCH 1/2] Define a new remoteaccess HAL for test. Bug: 261108682 Test: Locally build and run Change-Id: I40f97f4b10714096a32c5b14c8716b6c8e413419 --- .../remoteaccess/hal/default/Android.bp | 25 ++++++++++++++++--- .../default/remoteaccess-tcu-test-service.rc | 4 +++ 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 automotive/remoteaccess/hal/default/remoteaccess-tcu-test-service.rc diff --git a/automotive/remoteaccess/hal/default/Android.bp b/automotive/remoteaccess/hal/default/Android.bp index a2bf86c9e5..f27b8f86e6 100644 --- a/automotive/remoteaccess/hal/default/Android.bp +++ b/automotive/remoteaccess/hal/default/Android.bp @@ -18,11 +18,9 @@ package { default_applicable_licenses: ["Android-Apache-2.0"], } -cc_binary { - name: "android.hardware.automotive.remoteaccess@V1-default-service", +cc_defaults { + name: "remote-access-hal-defaults", vendor: true, - vintf_fragments: ["remoteaccess-default-service.xml"], - init_rc: ["remoteaccess-default-service.rc"], relative_install_path: "hw", srcs: ["src/RemoteAccessImpl.cpp"], whole_static_libs: [ @@ -41,10 +39,29 @@ cc_binary { ], cflags: [ "-Wno-unused-parameter", + ], +} + +cc_binary { + name: "android.hardware.automotive.remoteaccess@V1-default-service", + defaults: ["remote-access-hal-defaults"], + vintf_fragments: ["remoteaccess-default-service.xml"], + init_rc: ["remoteaccess-default-service.rc"], + cflags: [ "-DGRPC_SERVICE_ADDRESS=\"localhost:50051\"", ], } +cc_binary { + name: "android.hardware.automotive.remoteaccess@V1-tcu-test-service", + defaults: ["remote-access-hal-defaults"], + vintf_fragments: ["remoteaccess-default-service.xml"], + init_rc: ["remoteaccess-tcu-test-service.rc"], + cflags: [ + "-DGRPC_SERVICE_ADDRESS=\"10.10.10.1:50051\"", + ], +} + cc_library { name: "RemoteAccessService", vendor_available: true, diff --git a/automotive/remoteaccess/hal/default/remoteaccess-tcu-test-service.rc b/automotive/remoteaccess/hal/default/remoteaccess-tcu-test-service.rc new file mode 100644 index 0000000000..6437d70f08 --- /dev/null +++ b/automotive/remoteaccess/hal/default/remoteaccess-tcu-test-service.rc @@ -0,0 +1,4 @@ +service vendor.remoteaccess-default /vendor/bin/hw/android.hardware.automotive.remoteaccess@V1-tcu-test-service + class hal + user vehicle_network + group system inet From df39d6e4504aac41b692a5c4d691d3e47b4992f8 Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Fri, 2 Dec 2022 17:01:57 -0800 Subject: [PATCH 2/2] 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 --- .../hal/default/src/RemoteAccessService.cpp | 5 ++++- .../remoteaccess/test_grpc_server/README.md | 22 +++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp b/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp index 5cd58d3bba..4be30a20fa 100644 --- a/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp +++ b/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp @@ -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()); diff --git a/automotive/remoteaccess/test_grpc_server/README.md b/automotive/remoteaccess/test_grpc_server/README.md index 6bc1829388..af3d54ac5b 100644 --- a/automotive/remoteaccess/test_grpc_server/README.md +++ b/automotive/remoteaccess/test_grpc_server/README.md @@ -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`