From f3cb5562b22f587c8ec238bcc4331b82f4370d53 Mon Sep 17 00:00:00 2001
From: Yu-Han Yang <yuhany@google.com>
Date: Thu, 5 Mar 2020 22:41:23 -0800
Subject: [PATCH] Fix deadlock in cuttlefish/default implementation

Bug: 150830099

Test: atest LocationManagerFineTest#testRegisterGnssMeasurementsCallback
-c --iterations 100

Change-Id: I70aec19a481781d924ed3008765ca624a7eeb950
Merged-In: I70aec19a481781d924ed3008765ca624a7eeb950

(cherry picked from commit 3d652709ed40f8070c990bb275d3709d603d318e)
---
 gnss/2.0/default/GnssMeasurement.cpp | 2 +-
 gnss/2.0/default/GnssMeasurement.h   | 4 ++++
 gnss/2.1/default/GnssAntennaInfo.cpp | 2 +-
 gnss/2.1/default/GnssAntennaInfo.h   | 6 +++++-
 gnss/2.1/default/GnssMeasurement.cpp | 2 +-
 gnss/2.1/default/GnssMeasurement.h   | 6 ++++++
 6 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/gnss/2.0/default/GnssMeasurement.cpp b/gnss/2.0/default/GnssMeasurement.cpp
index d778d508cd..a3ea807729 100644
--- a/gnss/2.0/default/GnssMeasurement.cpp
+++ b/gnss/2.0/default/GnssMeasurement.cpp
@@ -49,8 +49,8 @@ Return<V1_0::IGnssMeasurement::GnssMeasurementStatus> GnssMeasurement::setCallba
 
 Return<void> GnssMeasurement::close() {
     ALOGD("close");
-    std::unique_lock<std::mutex> lock(mMutex);
     stop();
+    std::unique_lock<std::mutex> lock(mMutex);
     sCallback = nullptr;
     return Void();
 }
diff --git a/gnss/2.0/default/GnssMeasurement.h b/gnss/2.0/default/GnssMeasurement.h
index d8ffd59305..73eaa1320a 100644
--- a/gnss/2.0/default/GnssMeasurement.h
+++ b/gnss/2.0/default/GnssMeasurement.h
@@ -61,10 +61,14 @@ struct GnssMeasurement : public IGnssMeasurement {
     void stop();
     void reportMeasurement(const GnssData&);
 
+    // Guarded by mMutex
     static sp<IGnssMeasurementCallback> sCallback;
+
     std::atomic<long> mMinIntervalMillis;
     std::atomic<bool> mIsActive;
     std::thread mThread;
+
+    // Synchronization lock for sCallback
     mutable std::mutex mMutex;
 };
 
diff --git a/gnss/2.1/default/GnssAntennaInfo.cpp b/gnss/2.1/default/GnssAntennaInfo.cpp
index d32a0afc48..ed183a9383 100644
--- a/gnss/2.1/default/GnssAntennaInfo.cpp
+++ b/gnss/2.1/default/GnssAntennaInfo.cpp
@@ -55,8 +55,8 @@ Return<GnssAntennaInfo::GnssAntennaInfoStatus> GnssAntennaInfo::setCallback(
 
 Return<void> GnssAntennaInfo::close() {
     ALOGD("close");
-    std::unique_lock<std::mutex> lock(mMutex);
     stop();
+    std::unique_lock<std::mutex> lock(mMutex);
     sCallback = nullptr;
     return Void();
 }
diff --git a/gnss/2.1/default/GnssAntennaInfo.h b/gnss/2.1/default/GnssAntennaInfo.h
index f4bfd24367..94b2111d06 100644
--- a/gnss/2.1/default/GnssAntennaInfo.h
+++ b/gnss/2.1/default/GnssAntennaInfo.h
@@ -47,10 +47,14 @@ struct GnssAntennaInfo : public IGnssAntennaInfo {
     void reportAntennaInfo(
             const hidl_vec<IGnssAntennaInfoCallback::GnssAntennaInfo>& antennaInfo) const;
 
+    // Guarded by mMutex
     static sp<IGnssAntennaInfoCallback> sCallback;
+
     std::atomic<long> mMinIntervalMillis;
     std::atomic<bool> mIsActive;
     std::thread mThread;
+
+    // Synchronization lock for sCallback
     mutable std::mutex mMutex;
 };
 
@@ -60,4 +64,4 @@ struct GnssAntennaInfo : public IGnssAntennaInfo {
 }  // namespace hardware
 }  // namespace android
 
-#endif  // ANDROID_HARDWARE_GNSS_V2_1_GNSSCONFIGURATION_H
\ No newline at end of file
+#endif  // ANDROID_HARDWARE_GNSS_V2_1_GNSSCONFIGURATION_H
diff --git a/gnss/2.1/default/GnssMeasurement.cpp b/gnss/2.1/default/GnssMeasurement.cpp
index 34e20e5790..63bbc0a399 100644
--- a/gnss/2.1/default/GnssMeasurement.cpp
+++ b/gnss/2.1/default/GnssMeasurement.cpp
@@ -47,8 +47,8 @@ Return<V1_0::IGnssMeasurement::GnssMeasurementStatus> GnssMeasurement::setCallba
 
 Return<void> GnssMeasurement::close() {
     ALOGD("close");
-    std::unique_lock<std::mutex> lock(mMutex);
     stop();
+    std::unique_lock<std::mutex> lock(mMutex);
     sCallback_2_1 = nullptr;
     sCallback_2_0 = nullptr;
     return Void();
diff --git a/gnss/2.1/default/GnssMeasurement.h b/gnss/2.1/default/GnssMeasurement.h
index 3ed7bc597d..d44641978f 100644
--- a/gnss/2.1/default/GnssMeasurement.h
+++ b/gnss/2.1/default/GnssMeasurement.h
@@ -66,11 +66,17 @@ struct GnssMeasurement : public IGnssMeasurement {
     void reportMeasurement(const GnssDataV2_0&);
     void reportMeasurement(const GnssDataV2_1&);
 
+    // Guarded by mMutex
     static sp<V2_1::IGnssMeasurementCallback> sCallback_2_1;
+
+    // Guarded by mMutex
     static sp<V2_0::IGnssMeasurementCallback> sCallback_2_0;
+
     std::atomic<long> mMinIntervalMillis;
     std::atomic<bool> mIsActive;
     std::thread mThread;
+
+    // Synchronization lock for sCallback_2_1 and sCallback_2_0
     mutable std::mutex mMutex;
 };