From 54d6a878fd762775a713e26b17cb2819039a050d Mon Sep 17 00:00:00 2001
From: Dante Russo <drusso@codeaurora.org>
Date: Tue, 8 Aug 2017 14:48:13 -0700
Subject: [PATCH] Fix potential deadlock in measurements report

If Gnss Measurements callback is called at the same time
as Gnss Measurements close is called, this will cause
dead lock.
Remove using mutex in this case and instead use a simple
bool to know whether or not to call into a the callback.

Bug: 63909793
Change-Id: I98d2be5ebd2ee0f36e4d2e31d20a74585089fe31
CRs-fixed: 2089716
---
 android/location_api/MeasurementAPIClient.cpp | 54 +++++--------------
 android/location_api/MeasurementAPIClient.h   |  8 +--
 2 files changed, 13 insertions(+), 49 deletions(-)

diff --git a/android/location_api/MeasurementAPIClient.cpp b/android/location_api/MeasurementAPIClient.cpp
index e3ae9ad3..dd7ceac5 100644
--- a/android/location_api/MeasurementAPIClient.cpp
+++ b/android/location_api/MeasurementAPIClient.cpp
@@ -50,25 +50,14 @@ static void convertGnssClock(GnssMeasurementsClock& in, IGnssMeasurementCallback
 
 MeasurementAPIClient::MeasurementAPIClient() :
     mGnssMeasurementCbIface(nullptr),
-    mLocationCapabilitiesMask(0)
+    mTracking(false)
 {
     LOC_LOGD("%s]: ()", __FUNCTION__);
-    pthread_mutex_init(&mLock, nullptr);
-    pthread_cond_init (&mCond, nullptr);
-
-    // set default LocationOptions.
-    memset(&mLocationOptions, 0, sizeof(LocationOptions));
-    mLocationOptions.size = sizeof(LocationOptions);
-    mLocationOptions.minInterval = 1000;
-    mLocationOptions.minDistance = 0;
-    mLocationOptions.mode = GNSS_SUPL_MODE_STANDALONE;
 }
 
 MeasurementAPIClient::~MeasurementAPIClient()
 {
     LOC_LOGD("%s]: ()", __FUNCTION__);
-    pthread_cond_destroy(&mCond);
-    pthread_mutex_destroy(&mLock);
 }
 
 // for GpsInterface
@@ -101,19 +90,14 @@ MeasurementAPIClient::measurementSetCallback(const sp<IGnssMeasurementCallback>&
     }
 
     locAPISetCallbacks(locationCallbacks);
-
-    while (!mLocationCapabilitiesMask) {
-        LOC_LOGD("%s]: wait for capabilities...", __FUNCTION__);
-        pthread_mutex_lock(&mLock);
-        pthread_cond_wait(&mCond, &mLock);
-        pthread_mutex_unlock(&mLock);
-    }
-    if (mLocationCapabilitiesMask & LOCATION_CAPABILITIES_GNSS_MSB_BIT)
-        mLocationOptions.mode = GNSS_SUPL_MODE_MSB;
-    else
-        mLocationOptions.mode = GNSS_SUPL_MODE_STANDALONE;
+    LocationOptions options;
+    memset(&options, 0, sizeof(LocationOptions));
+    options.size = sizeof(LocationOptions);
+    options.minInterval = 1000;
+    options.mode = GNSS_SUPL_MODE_STANDALONE;
+    mTracking = true;
     LOC_LOGD("%s]: start tracking session", __FUNCTION__);
-    locAPIStartTracking(mLocationOptions);
+    locAPIStartTracking(options);
 
     return IGnssMeasurement::GnssMeasurementStatus::SUCCESS;
 }
@@ -121,30 +105,17 @@ MeasurementAPIClient::measurementSetCallback(const sp<IGnssMeasurementCallback>&
 // for GpsMeasurementInterface
 void MeasurementAPIClient::measurementClose() {
     LOC_LOGD("%s]: ()", __FUNCTION__);
-    pthread_mutex_lock(&mLock);
-    mGnssMeasurementCbIface = nullptr;
-    pthread_mutex_unlock(&mLock);
+    mTracking = false;
     locAPIStopTracking();
 }
 
 // callbacks
-void MeasurementAPIClient::onCapabilitiesCb(LocationCapabilitiesMask capabilitiesMask)
-{
-    LOC_LOGD("%s]: (%02x)", __FUNCTION__, capabilitiesMask);
-    mLocationCapabilitiesMask = capabilitiesMask;
-    pthread_mutex_lock(&mLock);
-    pthread_cond_signal(&mCond);
-    pthread_mutex_unlock(&mLock);
-}
-
 void MeasurementAPIClient::onGnssMeasurementsCb(
         GnssMeasurementsNotification gnssMeasurementsNotification)
 {
-    LOC_LOGD("%s]: (count: %zu)", __FUNCTION__, gnssMeasurementsNotification.count);
-    // we don't need to lock the mutext
-    // if mGnssMeasurementCbIface is set to nullptr
-    if (mGnssMeasurementCbIface != nullptr) {
-        pthread_mutex_lock(&mLock);
+    LOC_LOGD("%s]: (count: %zu active: %zu)",
+            __FUNCTION__, gnssMeasurementsNotification.count, mTracking);
+    if (mTracking) {
         if (mGnssMeasurementCbIface != nullptr) {
             IGnssMeasurementCallback::GnssData gnssData;
             convertGnssData(gnssMeasurementsNotification, gnssData);
@@ -154,7 +125,6 @@ void MeasurementAPIClient::onGnssMeasurementsCb(
                     __func__, r.description().c_str());
             }
         }
-        pthread_mutex_unlock(&mLock);
     }
 }
 
diff --git a/android/location_api/MeasurementAPIClient.h b/android/location_api/MeasurementAPIClient.h
index e2021926..422564d4 100644
--- a/android/location_api/MeasurementAPIClient.h
+++ b/android/location_api/MeasurementAPIClient.h
@@ -59,18 +59,12 @@ public:
     void measurementClose();
 
     // callbacks we are interested in
-    void onCapabilitiesCb(LocationCapabilitiesMask capabilitiesMask) final;
     void onGnssMeasurementsCb(GnssMeasurementsNotification gnssMeasurementsNotification) final;
 
 private:
-    pthread_mutex_t mLock;
-    pthread_cond_t mCond;
-
     sp<IGnssMeasurementCallback> mGnssMeasurementCbIface;
 
-    LocationCapabilitiesMask mLocationCapabilitiesMask;
-
-    LocationOptions mLocationOptions;
+    bool mTracking;
 };
 
 }  // namespace implementation