commit 52cde3b8f704f7184a09f681ac588f3cbbc6c111
author Dante Russo <drusso@codeaurora.org> Fri Jul 24 15:46:44 2020 +0530
committer Lakshmi Prasanna <tlakshmi@codeaurora.org> Fri Jul 24 15:52:05 2020 +0530
tree a20f61e93de6032715a40ea62699f0f3eb15ecd7
parent 5fb7f69f477236db58d35b747aa5ccfd0245cc8a

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

msm8998/android/location_api/MeasurementAPIClient.cpp

diff --git a/msm8998/android/location_api/MeasurementAPIClient.cpp b/msm8998/android/location_api/MeasurementAPIClient.cpp
index 394a45f..075ef69 100644
--- a/msm8998/android/location_api/MeasurementAPIClient.cpp
+++ b/msm8998/android/location_api/MeasurementAPIClient.cpp
@@ -50,25 +50,14 @@
 
 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
@@ -100,19 +89,14 @@
     }
 
     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;
 }
@@ -120,30 +104,17 @@
 // 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);
@@ -153,7 +124,6 @@
                     __func__, r.description().c_str());
             }
         }
-        pthread_mutex_unlock(&mLock);
     }
 }
 

msm8998/android/location_api/MeasurementAPIClient.h

diff --git a/msm8998/android/location_api/MeasurementAPIClient.h b/msm8998/android/location_api/MeasurementAPIClient.h
index e202192..422564d 100644
--- a/msm8998/android/location_api/MeasurementAPIClient.h
+++ b/msm8998/android/location_api/MeasurementAPIClient.h
@@ -59,18 +59,12 @@
     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