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
This commit is contained in:
Dante Russo 2017-08-08 14:48:13 -07:00
parent 7cc7da646c
commit 54d6a878fd
2 changed files with 13 additions and 49 deletions

View file

@ -50,25 +50,14 @@ static void convertGnssClock(GnssMeasurementsClock& in, IGnssMeasurementCallback
MeasurementAPIClient::MeasurementAPIClient() : MeasurementAPIClient::MeasurementAPIClient() :
mGnssMeasurementCbIface(nullptr), mGnssMeasurementCbIface(nullptr),
mLocationCapabilitiesMask(0) mTracking(false)
{ {
LOC_LOGD("%s]: ()", __FUNCTION__); 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() MeasurementAPIClient::~MeasurementAPIClient()
{ {
LOC_LOGD("%s]: ()", __FUNCTION__); LOC_LOGD("%s]: ()", __FUNCTION__);
pthread_cond_destroy(&mCond);
pthread_mutex_destroy(&mLock);
} }
// for GpsInterface // for GpsInterface
@ -101,19 +90,14 @@ MeasurementAPIClient::measurementSetCallback(const sp<IGnssMeasurementCallback>&
} }
locAPISetCallbacks(locationCallbacks); locAPISetCallbacks(locationCallbacks);
LocationOptions options;
while (!mLocationCapabilitiesMask) { memset(&options, 0, sizeof(LocationOptions));
LOC_LOGD("%s]: wait for capabilities...", __FUNCTION__); options.size = sizeof(LocationOptions);
pthread_mutex_lock(&mLock); options.minInterval = 1000;
pthread_cond_wait(&mCond, &mLock); options.mode = GNSS_SUPL_MODE_STANDALONE;
pthread_mutex_unlock(&mLock); mTracking = true;
}
if (mLocationCapabilitiesMask & LOCATION_CAPABILITIES_GNSS_MSB_BIT)
mLocationOptions.mode = GNSS_SUPL_MODE_MSB;
else
mLocationOptions.mode = GNSS_SUPL_MODE_STANDALONE;
LOC_LOGD("%s]: start tracking session", __FUNCTION__); LOC_LOGD("%s]: start tracking session", __FUNCTION__);
locAPIStartTracking(mLocationOptions); locAPIStartTracking(options);
return IGnssMeasurement::GnssMeasurementStatus::SUCCESS; return IGnssMeasurement::GnssMeasurementStatus::SUCCESS;
} }
@ -121,30 +105,17 @@ MeasurementAPIClient::measurementSetCallback(const sp<IGnssMeasurementCallback>&
// for GpsMeasurementInterface // for GpsMeasurementInterface
void MeasurementAPIClient::measurementClose() { void MeasurementAPIClient::measurementClose() {
LOC_LOGD("%s]: ()", __FUNCTION__); LOC_LOGD("%s]: ()", __FUNCTION__);
pthread_mutex_lock(&mLock); mTracking = false;
mGnssMeasurementCbIface = nullptr;
pthread_mutex_unlock(&mLock);
locAPIStopTracking(); locAPIStopTracking();
} }
// callbacks // 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( void MeasurementAPIClient::onGnssMeasurementsCb(
GnssMeasurementsNotification gnssMeasurementsNotification) GnssMeasurementsNotification gnssMeasurementsNotification)
{ {
LOC_LOGD("%s]: (count: %zu)", __FUNCTION__, gnssMeasurementsNotification.count); LOC_LOGD("%s]: (count: %zu active: %zu)",
// we don't need to lock the mutext __FUNCTION__, gnssMeasurementsNotification.count, mTracking);
// if mGnssMeasurementCbIface is set to nullptr if (mTracking) {
if (mGnssMeasurementCbIface != nullptr) {
pthread_mutex_lock(&mLock);
if (mGnssMeasurementCbIface != nullptr) { if (mGnssMeasurementCbIface != nullptr) {
IGnssMeasurementCallback::GnssData gnssData; IGnssMeasurementCallback::GnssData gnssData;
convertGnssData(gnssMeasurementsNotification, gnssData); convertGnssData(gnssMeasurementsNotification, gnssData);
@ -154,7 +125,6 @@ void MeasurementAPIClient::onGnssMeasurementsCb(
__func__, r.description().c_str()); __func__, r.description().c_str());
} }
} }
pthread_mutex_unlock(&mLock);
} }
} }

View file

@ -59,18 +59,12 @@ public:
void measurementClose(); void measurementClose();
// callbacks we are interested in // callbacks we are interested in
void onCapabilitiesCb(LocationCapabilitiesMask capabilitiesMask) final;
void onGnssMeasurementsCb(GnssMeasurementsNotification gnssMeasurementsNotification) final; void onGnssMeasurementsCb(GnssMeasurementsNotification gnssMeasurementsNotification) final;
private: private:
pthread_mutex_t mLock;
pthread_cond_t mCond;
sp<IGnssMeasurementCallback> mGnssMeasurementCbIface; sp<IGnssMeasurementCallback> mGnssMeasurementCbIface;
LocationCapabilitiesMask mLocationCapabilitiesMask; bool mTracking;
LocationOptions mLocationOptions;
}; };
} // namespace implementation } // namespace implementation