Fix intermittant crashes in location

Deleting memory without setting it to null
can cause the memory to still be non-null
and be interpreted as valid.
Also check if the entry is in the map using
find instead of assuming it is in the map.

Bug: 62033719 62033834 62033690 62033563 62032790
CRs-fixed: 2050837
Change-Id: I2534de2d6157be86ac95cfe6615e4c0019ee48fd

1 files changed, 126 insertions, 72 deletions

diff --git a/location/LocationAPIClientBase.cpp b/location/LocationAPIClientBase.cpp
index 5078313..58f7fcb 100644
--- a/location/LocationAPIClientBase.cpp
+++ b/location/LocationAPIClientBase.cpp
@@ -60,6 +60,8 @@ LocationAPIClientBase::LocationAPIClientBase() :
 
 void LocationAPIClientBase::locAPISetCallbacks(LocationCallbacks& locationCallbacks)
 {
+    pthread_mutex_lock(&mMutex);
+
     if (locationCallbacks.geofenceBreachCb != nullptr) {
         mGeofenceBreachCallback = locationCallbacks.geofenceBreachCb;
         locationCallbacks.geofenceBreachCb =
@@ -101,10 +103,14 @@ void LocationAPIClientBase::locAPISetCallbacks(LocationCallbacks& locationCallba
 
         mLocationControlAPI = LocationControlAPI::createInstance(locationControlCallbacks);
     }
+
+    pthread_mutex_unlock(&mMutex);
 }
 
 LocationAPIClientBase::~LocationAPIClientBase()
 {
+    pthread_mutex_lock(&mMutex);
+
     if (mLocationAPI) {
         mLocationAPI->destroy();
         mLocationAPI = nullptr;
@@ -114,7 +120,6 @@ LocationAPIClientBase::~LocationAPIClientBase()
         mLocationControlAPI = nullptr;
     }
 
-    pthread_mutex_lock(&mMutex);
     for (int i = 0; i < REQUEST_MAX; i++) {
         if (mRequestQueues[i]) {
             delete mRequestQueues[i];
@@ -129,11 +134,12 @@ LocationAPIClientBase::~LocationAPIClientBase()
 uint32_t LocationAPIClientBase::locAPIStartTracking(LocationOptions& options)
 {
     uint32_t retVal = LOCATION_ERROR_GENERAL_FAILURE;
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_TRACKING];
         if (requests) {
             delete requests;
+            requests = nullptr;
         }
         uint32_t session = mLocationAPI->startTracking(options);
         LOC_LOGI("%s:%d] start new session: %d", __FUNCTION__, __LINE__, session);
@@ -143,42 +149,46 @@ uint32_t LocationAPIClientBase::locAPIStartTracking(LocationOptions& options)
         requests = new RequestQueue(session);
         requests->push(new StartTrackingRequest(*this));
         mRequestQueues[REQUEST_TRACKING] = requests;
-        pthread_mutex_unlock(&mMutex);
 
         retVal = LOCATION_ERROR_SUCCESS;
     }
+    pthread_mutex_unlock(&mMutex);
 
     return retVal;
 }
 
 void LocationAPIClientBase::locAPIStopTracking()
 {
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
-        pthread_mutex_lock(&mMutex);
-        uint32_t session = -1;
+        uint32_t session = 0;
         RequestQueue* requests = mRequestQueues[REQUEST_TRACKING];
         if (requests) {
             session = requests->getSession();
-            requests->push(new StopTrackingRequest(*this));
-            mLocationAPI->stopTracking(session);
+            if (session > 0) {
+                requests->push(new StopTrackingRequest(*this));
+                mLocationAPI->stopTracking(session);
+            }
         }
-        pthread_mutex_unlock(&mMutex);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 void LocationAPIClientBase::locAPIUpdateTrackingOptions(LocationOptions& options)
 {
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
-        pthread_mutex_lock(&mMutex);
-        uint32_t session = -1;
+        uint32_t session = 0;
         RequestQueue* requests = mRequestQueues[REQUEST_TRACKING];
         if (requests) {
             session = requests->getSession();
-            requests->push(new UpdateTrackingOptionsRequest(*this));
-            mLocationAPI->updateTrackingOptions(session, options);
+            if (session > 0) {
+                requests->push(new UpdateTrackingOptionsRequest(*this));
+                mLocationAPI->updateTrackingOptions(session, options);
+            }
         }
-        pthread_mutex_unlock(&mMutex);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 int32_t LocationAPIClientBase::locAPIGetBatchSize()
@@ -202,8 +212,8 @@ uint32_t LocationAPIClientBase::locAPIStartSession(uint32_t id, uint32_t session
         LocationOptions& options)
 {
     uint32_t retVal = LOCATION_ERROR_GENERAL_FAILURE;
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
-        pthread_mutex_lock(&mMutex);
 
         if (mSessionMap.find(id) != mSessionMap.end()) {
             LOC_LOGE("%s:%d] session %d has already started.", __FUNCTION__, __LINE__, id);
@@ -216,6 +226,7 @@ uint32_t LocationAPIClientBase::locAPIStartSession(uint32_t id, uint32_t session
                 RequestQueue* requests = mRequestQueues[REQUEST_TRACKING];
                 if (requests) {
                     delete requests;
+                    requests = nullptr;
                 }
                 trackingSession = mLocationAPI->startTracking(options);
                 LOC_LOGI("%s:%d] start new session: %d", __FUNCTION__, __LINE__, trackingSession);
@@ -226,6 +237,7 @@ uint32_t LocationAPIClientBase::locAPIStartSession(uint32_t id, uint32_t session
                 RequestQueue* requests = mRequestQueues[REQUEST_BATCHING];
                 if (requests) {
                     delete requests;
+                    requests = nullptr;
                 }
                 batchingSession = mLocationAPI->startBatching(options);
                 LOC_LOGI("%s:%d] start new session: %d", __FUNCTION__, __LINE__, batchingSession);
@@ -244,8 +256,8 @@ uint32_t LocationAPIClientBase::locAPIStartSession(uint32_t id, uint32_t session
             retVal = LOCATION_ERROR_SUCCESS;
         }
 
-        pthread_mutex_unlock(&mMutex);
     }
+    pthread_mutex_unlock(&mMutex);
 
     return retVal;
 }
@@ -253,8 +265,8 @@ uint32_t LocationAPIClientBase::locAPIStartSession(uint32_t id, uint32_t session
 uint32_t LocationAPIClientBase::locAPIStopSession(uint32_t id)
 {
     uint32_t retVal = LOCATION_ERROR_GENERAL_FAILURE;
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
-        pthread_mutex_lock(&mMutex);
 
         if (mSessionMap.find(id) != mSessionMap.end()) {
             SessionEntity entity = mSessionMap[id];
@@ -285,8 +297,8 @@ uint32_t LocationAPIClientBase::locAPIStopSession(uint32_t id)
             LOC_LOGE("%s:%d] session %d is not exist.", __FUNCTION__, __LINE__, id);
         }
 
-        pthread_mutex_unlock(&mMutex);
     }
+    pthread_mutex_unlock(&mMutex);
     return retVal;
 }
 
@@ -294,8 +306,8 @@ uint32_t LocationAPIClientBase::locAPIUpdateSessionOptions(uint32_t id, uint32_t
         LocationOptions& options)
 {
     uint32_t retVal = LOCATION_ERROR_GENERAL_FAILURE;
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
-        pthread_mutex_lock(&mMutex);
 
         if (mSessionMap.find(id) != mSessionMap.end()) {
             SessionEntity& entity = mSessionMap[id];
@@ -326,6 +338,7 @@ uint32_t LocationAPIClientBase::locAPIUpdateSessionOptions(uint32_t id, uint32_t
                         RequestQueue* requests = mRequestQueues[REQUEST_TRACKING];
                         if (requests) {
                             delete requests;
+                            requests = nullptr;
                         }
                         trackingSession = mLocationAPI->startTracking(options);
                         LOC_LOGI("%s:%d] start new session: %d",
@@ -351,6 +364,7 @@ uint32_t LocationAPIClientBase::locAPIUpdateSessionOptions(uint32_t id, uint32_t
                         RequestQueue* requests = mRequestQueues[REQUEST_BATCHING];
                         if (requests) {
                             delete requests;
+                            requests = nullptr;
                         }
                         batchingSession = mLocationAPI->startBatching(options);
                         LOC_LOGI("%s:%d] start new session: %d",
@@ -379,32 +393,34 @@ uint32_t LocationAPIClientBase::locAPIUpdateSessionOptions(uint32_t id, uint32_t
             LOC_LOGE("%s:%d] session %d is not exist.", __FUNCTION__, __LINE__, id);
         }
 
-        pthread_mutex_unlock(&mMutex);
     }
+    pthread_mutex_unlock(&mMutex);
     return retVal;
 }
 
 void LocationAPIClientBase::locAPIGetBatchedLocations(size_t count)
 {
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
-        pthread_mutex_lock(&mMutex);
-        uint32_t session = -1;
+        uint32_t session = 0;
         RequestQueue* requests = mRequestQueues[REQUEST_BATCHING];
         if (requests) {
             session = requests->getSession();
-            requests->push(new GetBatchedLocationsRequest(*this));
-            mLocationAPI->getBatchedLocations(session, count);
+            if (session > 0) {
+                requests->push(new GetBatchedLocationsRequest(*this));
+                mLocationAPI->getBatchedLocations(session, count);
+            }
         }
-        pthread_mutex_unlock(&mMutex);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 uint32_t LocationAPIClientBase::locAPIAddGeofences(
         size_t count, uint32_t* ids, GeofenceOption* options, GeofenceInfo* data)
 {
     uint32_t retVal = LOCATION_ERROR_GENERAL_FAILURE;
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_GEOFENCE];
         if (!requests) {
             // Create a new RequestQueue for Geofenceing if we've not had one.
@@ -422,128 +438,158 @@ uint32_t LocationAPIClientBase::locAPIAddGeofences(
             }
             retVal = LOCATION_ERROR_SUCCESS;
         }
-        pthread_mutex_unlock(&mMutex);
     }
+    pthread_mutex_unlock(&mMutex);
 
     return retVal;
 }
 
 void LocationAPIClientBase::locAPIRemoveGeofences(size_t count, uint32_t* ids)
 {
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
         uint32_t* sessions = (uint32_t*)malloc(sizeof(uint32_t) * count);
 
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_GEOFENCE];
         if (requests) {
+            size_t j = 0;
             for (size_t i = 0; i < count; i++) {
-                sessions[i] = mGeofenceBiDict.getSession(ids[i]);
+                sessions[j] = mGeofenceBiDict.getSession(ids[i]);
+                if (sessions[j] > 0) {
+                    j++;
+                }
+            }
+            if (j > 0) {
+                requests->push(new RemoveGeofencesRequest(*this));
+                mLocationAPI->removeGeofences(j, sessions);
             }
-            requests->push(new RemoveGeofencesRequest(*this));
-            mLocationAPI->removeGeofences(count, sessions);
         }
-        pthread_mutex_unlock(&mMutex);
 
         free(sessions);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 void LocationAPIClientBase::locAPIModifyGeofences(
         size_t count, uint32_t* ids, GeofenceOption* options)
 {
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
         uint32_t* sessions = (uint32_t*)malloc(sizeof(uint32_t) * count);
 
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_GEOFENCE];
         if (requests) {
+            size_t j = 0;
             for (size_t i = 0; i < count; i++) {
-                sessions[i] = mGeofenceBiDict.getSession(ids[i]);
-                mGeofenceBiDict.set(ids[i], sessions[i], options[i].breachTypeMask);
+                sessions[j] = mGeofenceBiDict.getSession(ids[i]);
+                if (sessions[j] > 0) {
+                    mGeofenceBiDict.set(ids[i], sessions[j], options[i].breachTypeMask);
+                    j++;
+                }
+            }
+            if (j > 0) {
+                requests->push(new ModifyGeofencesRequest(*this));
+                mLocationAPI->modifyGeofences(j, sessions, options);
             }
-            requests->push(new ModifyGeofencesRequest(*this));
-            mLocationAPI->modifyGeofences(count, sessions, options);
         }
-        pthread_mutex_unlock(&mMutex);
 
         free(sessions);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 void LocationAPIClientBase::locAPIPauseGeofences(size_t count, uint32_t* ids)
 {
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
         uint32_t* sessions = (uint32_t*)malloc(sizeof(uint32_t) * count);
 
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_GEOFENCE];
         if (requests) {
+            size_t j = 0;
             for (size_t i = 0; i < count; i++) {
-                sessions[i] = mGeofenceBiDict.getSession(ids[i]);
+                sessions[j] = mGeofenceBiDict.getSession(ids[i]);
+                if (sessions[j] > 0) {
+                    j++;
+                }
+            }
+            if (j > 0) {
+                requests->push(new PauseGeofencesRequest(*this));
+                mLocationAPI->pauseGeofences(j, sessions);
             }
-            requests->push(new PauseGeofencesRequest(*this));
-            mLocationAPI->pauseGeofences(count, sessions);
         }
-        pthread_mutex_unlock(&mMutex);
 
         free(sessions);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 void LocationAPIClientBase::locAPIResumeGeofences(
         size_t count, uint32_t* ids, GeofenceBreachTypeMask* mask)
 {
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
         uint32_t* sessions = (uint32_t*)malloc(sizeof(uint32_t) * count);
 
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_GEOFENCE];
         if (requests) {
+            size_t j = 0;
             for (size_t i = 0; i < count; i++) {
-                sessions[i] = mGeofenceBiDict.getSession(ids[i]);
-                if (mask) {
-                    mGeofenceBiDict.set(ids[i], sessions[i], mask[i]);
+                sessions[j] = mGeofenceBiDict.getSession(ids[i]);
+                if (sessions[j] > 0) {
+                    if (mask) {
+                        mGeofenceBiDict.set(ids[i], sessions[j], mask[i]);
+                    }
+                    j++;
                 }
             }
-            requests->push(new ResumeGeofencesRequest(*this));
-            mLocationAPI->resumeGeofences(count, sessions);
+            if (j > 0) {
+                requests->push(new ResumeGeofencesRequest(*this));
+                mLocationAPI->resumeGeofences(j, sessions);
+            }
         }
-        pthread_mutex_unlock(&mMutex);
 
         free(sessions);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 void LocationAPIClientBase::locAPIRemoveAllGeofences()
 {
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
         std::vector<uint32_t> sessionsVec = mGeofenceBiDict.getAllSessions();
         size_t count = sessionsVec.size();
         uint32_t* sessions = (uint32_t*)malloc(sizeof(uint32_t) * count);
 
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_GEOFENCE];
         if (requests) {
+            size_t j = 0;
             for (size_t i = 0; i < count; i++) {
-                sessions[i] = sessionsVec[i];
+                sessions[j] = sessionsVec[i];
+                if (sessions[j] > 0) {
+                    j++;
+                }
+            }
+            if (j > 0) {
+                requests->push(new RemoveGeofencesRequest(*this));
+                mLocationAPI->removeGeofences(j, sessions);
             }
-            requests->push(new RemoveGeofencesRequest(*this));
-            mLocationAPI->removeGeofences(count, sessions);
         }
-        pthread_mutex_unlock(&mMutex);
 
         free(sessions);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 void LocationAPIClientBase::locAPIGnssNiResponse(uint32_t id, GnssNiResponse response)
 {
-    uint32_t session = 0;
+    pthread_mutex_lock(&mMutex);
     if (mLocationAPI) {
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_NIRESPONSE];
         if (requests) {
             delete requests;
+            requests = nullptr;
         }
         uint32_t session = id;
         mLocationAPI->gnssNiResponse(id, response);
@@ -551,28 +597,29 @@ void LocationAPIClientBase::locAPIGnssNiResponse(uint32_t id, GnssNiResponse res
         requests = new RequestQueue(session);
         requests->push(new GnssNiResponseRequest(*this));
         mRequestQueues[REQUEST_NIRESPONSE] = requests;
-        pthread_mutex_unlock(&mMutex);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 uint32_t LocationAPIClientBase::locAPIGnssDeleteAidingData(GnssAidingData& data)
 {
     uint32_t retVal = LOCATION_ERROR_GENERAL_FAILURE;
+    pthread_mutex_lock(&mMutex);
     if (mLocationControlAPI) {
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_DELETEAIDINGDATA];
         if (requests) {
             delete requests;
+            requests = nullptr;
         }
         uint32_t session = mLocationControlAPI->gnssDeleteAidingData(data);
         LOC_LOGI("%s:%d] start new session: %d", __FUNCTION__, __LINE__, session);
         requests = new RequestQueue(session);
         requests->push(new GnssDeleteAidingDataRequest(*this));
         mRequestQueues[REQUEST_DELETEAIDINGDATA] = requests;
-        pthread_mutex_unlock(&mMutex);
 
         retVal = LOCATION_ERROR_SUCCESS;
     }
+    pthread_mutex_unlock(&mMutex);
 
     return retVal;
 }
@@ -580,38 +627,41 @@ uint32_t LocationAPIClientBase::locAPIGnssDeleteAidingData(GnssAidingData& data)
 uint32_t LocationAPIClientBase::locAPIEnable(LocationTechnologyType techType)
 {
     uint32_t retVal = LOCATION_ERROR_GENERAL_FAILURE;
+    pthread_mutex_lock(&mMutex);
     if (mLocationControlAPI) {
-        pthread_mutex_lock(&mMutex);
         RequestQueue* requests = mRequestQueues[REQUEST_CONTROL];
         if (requests) {
             delete requests;
+            requests = nullptr;
         }
         uint32_t session = mLocationControlAPI->enable(techType);
         LOC_LOGI("%s:%d] start new session: %d", __FUNCTION__, __LINE__, session);
         requests = new RequestQueue(session);
         requests->push(new EnableRequest(*this));
         mRequestQueues[REQUEST_CONTROL] = requests;
-        pthread_mutex_unlock(&mMutex);
 
         retVal = LOCATION_ERROR_SUCCESS;
     }
+    pthread_mutex_unlock(&mMutex);
 
     return retVal;
 }
 
 void LocationAPIClientBase::locAPIDisable()
 {
+    pthread_mutex_lock(&mMutex);
     if (mLocationControlAPI) {
-        pthread_mutex_lock(&mMutex);
-        uint32_t session = -1;
+        uint32_t session = 0;
         RequestQueue* requests = mRequestQueues[REQUEST_CONTROL];
         if (requests) {
             session = requests->getSession();
-            requests->push(new DisableRequest(*this));
-            mLocationControlAPI->disable(session);
+            if (session > 0) {
+                requests->push(new DisableRequest(*this));
+                mLocationControlAPI->disable(session);
+            }
         }
-        pthread_mutex_unlock(&mMutex);
     }
+    pthread_mutex_unlock(&mMutex);
 }
 
 uint32_t LocationAPIClientBase::locAPIGnssUpdateConfig(GnssConfig config)
@@ -622,23 +672,25 @@ uint32_t LocationAPIClientBase::locAPIGnssUpdateConfig(GnssConfig config)
         return retVal;
     }
 
+    pthread_mutex_lock(&mMutex);
     if (mLocationControlAPI) {
-        pthread_mutex_lock(&mMutex);
 
         memcpy(&mConfig, &config, sizeof(GnssConfig));
 
-        uint32_t session = -1;
+        uint32_t session = 0;
         RequestQueue* requests = mRequestQueues[REQUEST_CONTROL];
         if (requests) {
             session = requests->getSession();
-            requests->push(new GnssUpdateConfigRequest(*this));
-            uint32_t* idArray = mLocationControlAPI->gnssUpdateConfig(config);
-            LOC_LOGV("%s:%d] gnssUpdateConfig return array: %p", __FUNCTION__, __LINE__, idArray);
+            if (session > 0) {
+                requests->push(new GnssUpdateConfigRequest(*this));
+                uint32_t* idArray = mLocationControlAPI->gnssUpdateConfig(config);
+                LOC_LOGV("%s:%d] gnssUpdateConfig return array: %p", __FUNCTION__, __LINE__, idArray);
+            }
         }
-        pthread_mutex_unlock(&mMutex);
 
         retVal = LOCATION_ERROR_SUCCESS;
     }
+    pthread_mutex_unlock(&mMutex);
     return retVal;
 }
 
@@ -655,6 +707,8 @@ void LocationAPIClientBase::beforeGeofenceBreachCb(
     for (size_t i = 0; i < n; i++) {
         uint32_t id = mGeofenceBiDict.getId(geofenceBreachNotification.ids[i]);
         GeofenceBreachTypeMask type = mGeofenceBiDict.getType(geofenceBreachNotification.ids[i]);
+        // if type == 0, we will not head into the fllowing block anyway.
+        // so we don't need to check id and type
         if ((geofenceBreachNotification.type == GEOFENCE_BREACH_ENTER &&
             (type & GEOFENCE_BREACH_ENTER_BIT)) ||
             (geofenceBreachNotification.type == GEOFENCE_BREACH_EXIT &&