From e232986d84deb7178d1a57cc7f1e4d24cbaeeb3c Mon Sep 17 00:00:00 2001 From: Wei Chen Date: Thu, 15 Nov 2018 09:41:26 -0800 Subject: [PATCH] GPS location API: support destroy with callback for completion Support Location API destroy function with callback. Location API client that passes callback to Location API need to wait for the destroy complete callback to be invoked before releasing the memory that holds the callback Change-Id: I29b1c6d46feb79c789e6f1ec1500c941b022a3ac CRs-fixed: 2349398 --- gnss/GnssAdapter.cpp | 15 +++- gnss/GnssAdapter.h | 5 +- gnss/location_gnss.cpp | 8 +- location/LocationAPI.cpp | 148 ++++++++++++++++++++++++++++------ location/LocationAPI.h | 12 ++- location/LocationDataTypes.h | 3 + location/location_interface.h | 8 +- 7 files changed, 159 insertions(+), 40 deletions(-) diff --git a/gnss/GnssAdapter.cpp b/gnss/GnssAdapter.cpp index fc59c224..f7297d0e 100644 --- a/gnss/GnssAdapter.cpp +++ b/gnss/GnssAdapter.cpp @@ -1859,25 +1859,32 @@ GnssAdapter::addClientCommand(LocationAPI* client, const LocationCallbacks& call } void -GnssAdapter::removeClientCommand(LocationAPI* client) +GnssAdapter::removeClientCommand(LocationAPI* client, + removeClientCompleteCallback rmClientCb) { LOC_LOGD("%s]: client %p", __func__, client); struct MsgRemoveClient : public LocMsg { GnssAdapter& mAdapter; LocationAPI* mClient; + removeClientCompleteCallback mRmClientCb; inline MsgRemoveClient(GnssAdapter& adapter, - LocationAPI* client) : + LocationAPI* client, + removeClientCompleteCallback rmCb) : LocMsg(), mAdapter(adapter), - mClient(client) {} + mClient(client), + mRmClientCb(rmCb){} inline virtual void proc() const { mAdapter.stopClientSessions(mClient); mAdapter.eraseClient(mClient); + if (nullptr != mRmClientCb) { + mRmClientCb(mClient); + } } }; - sendMsg(new MsgRemoveClient(*this, client)); + sendMsg(new MsgRemoveClient(*this, client, rmClientCb)); } void diff --git a/gnss/GnssAdapter.h b/gnss/GnssAdapter.h index f382c6ad..1682f027 100644 --- a/gnss/GnssAdapter.h +++ b/gnss/GnssAdapter.h @@ -128,6 +128,8 @@ typedef std::function GnssEnergyConsumedCallback; +typedef void (*removeClientCompleteCallback)(LocationAPI* client); + class GnssAdapter : public LocAdapterBase { /* ==== Engine Hub ===================================================================== */ @@ -206,7 +208,8 @@ public: /* ==== CLIENT ========================================================================= */ /* ======== COMMANDS ====(Called from Client Thread)==================================== */ void addClientCommand(LocationAPI* client, const LocationCallbacks& callbacks); - void removeClientCommand(LocationAPI* client); + void removeClientCommand(LocationAPI* client, + removeClientCompleteCallback rmClientCb); void requestCapabilitiesCommand(LocationAPI* client); /* ======== UTILITIES ================================================================== */ void saveClient(LocationAPI* client, const LocationCallbacks& callbacks); diff --git a/gnss/location_gnss.cpp b/gnss/location_gnss.cpp index 5a7710a2..f9fbddc9 100644 --- a/gnss/location_gnss.cpp +++ b/gnss/location_gnss.cpp @@ -36,7 +36,7 @@ static void initialize(); static void deinitialize(); static void addClient(LocationAPI* client, const LocationCallbacks& callbacks); -static void removeClient(LocationAPI* client); +static void removeClient(LocationAPI* client, removeClientCompleteCallback rmClientCb); static void requestCapabilities(LocationAPI* client); static uint32_t startTracking(LocationAPI* client, TrackingOptions&); @@ -140,10 +140,10 @@ static void addClient(LocationAPI* client, const LocationCallbacks& callbacks) } } -static void removeClient(LocationAPI* client) +static void removeClient(LocationAPI* client, removeClientCompleteCallback rmClientCb) { if (NULL != gGnssAdapter) { - gGnssAdapter->removeClientCommand(client); + gGnssAdapter->removeClientCommand(client, rmClientCb); } } @@ -345,4 +345,4 @@ static void getGnssEnergyConsumed(GnssEnergyConsumedCallback energyConsumedCb) { if (NULL != gGnssAdapter) { gGnssAdapter->getGnssEnergyConsumedCommand(energyConsumedCb); } -} \ No newline at end of file +} diff --git a/location/LocationAPI.cpp b/location/LocationAPI.cpp index 9a67f39e..4d859b30 100644 --- a/location/LocationAPI.cpp +++ b/location/LocationAPI.cpp @@ -36,15 +36,37 @@ #include typedef void* (getLocationInterface)(); + +typedef uint16_t LocationAdapterTypeMask; +typedef enum { + LOCATION_ADAPTER_GNSS_TYPE_BIT = (1<<0), // adapter type is GNSS + LOCATION_ADAPTER_FLP_TYPE_BIT = (1<<1), // adapter type is FLP + LOCATION_ADAPTER_GEOFENCE_TYPE_BIT = (1<<2) // adapter type is geo fence +} LocationAdapterTypeBits; + +typedef struct { + // bit mask of the adpaters that we need to wait for the removeClientCompleteCallback + // before we invoke the registered locationApiDestroyCompleteCallback + LocationAdapterTypeMask waitAdapterMask; + locationApiDestroyCompleteCallback destroyCompleteCb; +} LocationAPIDestroyCbData; + +// This is the map for the client that has requested destroy with +// destroy callback provided. +typedef std::map + LocationClientDestroyCbMap; + typedef std::map LocationClientMap; typedef struct { LocationClientMap clientData; + LocationClientDestroyCbMap destroyClientData; LocationControlAPI* controlAPI; LocationControlCallbacks controlCallbacks; GnssInterface* gnssInterface; GeofenceInterface* geofenceInterface; FlpInterface* flpInterface; } LocationAPIData; + static LocationAPIData gData = {}; static pthread_mutex_t gDataMutex = PTHREAD_MUTEX_INITIALIZER; static bool gGnssLoadFailed = false; @@ -80,7 +102,8 @@ static bool isGeofenceClient(LocationCallbacks& locationCallbacks) locationCallbacks.geofenceStatusCb != nullptr); } -static void* loadLocationInterface(const char* library, const char* name) { +static void* loadLocationInterface(const char* library, const char* name) +{ LOC_LOGD("%s]: loading %s::%s ...", __func__, library, name); if (NULL == library || NULL == name) { return NULL; @@ -106,6 +129,46 @@ static void* loadLocationInterface(const char* library, const char* name) { } } +void onRemoveClientCompleteCb ( + LocationAPI* client, LocationAdapterTypeMask adapterType) +{ + bool invokeCallback = false; + locationApiDestroyCompleteCallback destroyCompleteCb; + LOC_LOGd("adatper type %x", adapterType); + pthread_mutex_lock(&gDataMutex); + auto it = gData.destroyClientData.find(client); + if (it != gData.destroyClientData.end()) { + it->second.waitAdapterMask &= ~adapterType; + if (it->second.waitAdapterMask == 0) { + invokeCallback = true; + destroyCompleteCb = it->second.destroyCompleteCb; + gData.destroyClientData.erase(it); + } + } + pthread_mutex_unlock(&gDataMutex); + + if ((true == invokeCallback) && (nullptr != destroyCompleteCb)) { + LOC_LOGd("invoke client destroy cb"); + (destroyCompleteCb) (); + LOC_LOGd("finish invoke client destroy cb"); + } +} + +void onGnssRemoveClientCompleteCb (LocationAPI* client) +{ + onRemoveClientCompleteCb (client, LOCATION_ADAPTER_GNSS_TYPE_BIT); +} + +void onFlpRemoveClientCompleteCb (LocationAPI* client) +{ + onRemoveClientCompleteCb (client, LOCATION_ADAPTER_FLP_TYPE_BIT); +} + +void onGeofenceRemoveClientCompleteCb (LocationAPI* client) +{ + onRemoveClientCompleteCb (client, LOCATION_ADAPTER_GEOFENCE_TYPE_BIT); +} + LocationAPI* LocationAPI::createInstance(LocationCallbacks& locationCallbacks) { @@ -188,9 +251,66 @@ LocationAPI::createInstance(LocationCallbacks& locationCallbacks) } void -LocationAPI::destroy() +LocationAPI::destroy(locationApiDestroyCompleteCallback destroyCompleteCb) { - delete this; + bool invokeDestroyCb = false; + + pthread_mutex_lock(&gDataMutex); + auto it = gData.clientData.find(this); + if (it != gData.clientData.end()) { + bool removeFromGnssInf = + (isGnssClient(it->second) && NULL != gData.gnssInterface); + bool removeFromFlpInf = + (isFlpClient(it->second) && NULL != gData.flpInterface); + bool removeFromGeofenceInf = + (isGeofenceClient(it->second) && NULL != gData.geofenceInterface); + bool needToWait = (removeFromGnssInf || removeFromFlpInf || removeFromGeofenceInf); + LOC_LOGe("removeFromGnssInf: %d, removeFromFlpInf: %d, removeFromGeofenceInf: %d, need %d", + removeFromGnssInf, removeFromFlpInf, removeFromGeofenceInf, needToWait); + + if ((NULL != destroyCompleteCb) && (true == needToWait)) { + LocationAPIDestroyCbData destroyCbData = {}; + destroyCbData.destroyCompleteCb = destroyCompleteCb; + // record down from which adapter we need to wait for the destroy complete callback + // only when we have received all the needed callbacks from all the associated stacks, + // we shall notify the client. + destroyCbData.waitAdapterMask = + (removeFromGnssInf ? LOCATION_ADAPTER_GNSS_TYPE_BIT : 0); + destroyCbData.waitAdapterMask |= + (removeFromFlpInf ? LOCATION_ADAPTER_FLP_TYPE_BIT : 0); + destroyCbData.waitAdapterMask |= + (removeFromGeofenceInf ? LOCATION_ADAPTER_GEOFENCE_TYPE_BIT : 0); + gData.destroyClientData[this] = destroyCbData; + LOC_LOGe("destroy data stored in the map: 0x%x", destroyCbData.waitAdapterMask); + } + + if (removeFromGnssInf) { + gData.gnssInterface->removeClient(it->first, + onGnssRemoveClientCompleteCb); + } + if (removeFromFlpInf) { + gData.flpInterface->removeClient(it->first, + onFlpRemoveClientCompleteCb); + } + if (removeFromGeofenceInf) { + gData.geofenceInterface->removeClient(it->first, + onGeofenceRemoveClientCompleteCb); + } + + gData.clientData.erase(it); + + if ((NULL != destroyCompleteCb) && (false == needToWait)) { + invokeDestroyCb = true; + } + } else { + LOC_LOGE("%s:%d]: Location API client %p not found in client data", + __func__, __LINE__, this); + } + + pthread_mutex_unlock(&gDataMutex); + if (invokeDestroyCb == true) { + (destroyCompleteCb) (); + } } LocationAPI::LocationAPI() @@ -198,29 +318,9 @@ LocationAPI::LocationAPI() LOC_LOGD("LOCATION API CONSTRUCTOR"); } +// private destructor LocationAPI::~LocationAPI() { - LOC_LOGD("LOCATION API DESTRUCTOR"); - pthread_mutex_lock(&gDataMutex); - - auto it = gData.clientData.find(this); - if (it != gData.clientData.end()) { - if (isGnssClient(it->second) && NULL != gData.gnssInterface) { - gData.gnssInterface->removeClient(it->first); - } - if (isFlpClient(it->second) && NULL != gData.flpInterface) { - gData.flpInterface->removeClient(it->first); - } - if (isGeofenceClient(it->second) && NULL != gData.geofenceInterface) { - gData.geofenceInterface->removeClient(it->first); - } - gData.clientData.erase(it); - } else { - LOC_LOGE("%s:%d]: Location API client %p not found in client data", - __func__, __LINE__, this); - } - - pthread_mutex_unlock(&gDataMutex); } void diff --git a/location/LocationAPI.h b/location/LocationAPI.h index f4f31f96..c270ef41 100644 --- a/location/LocationAPI.h +++ b/location/LocationAPI.h @@ -43,10 +43,14 @@ public: of instances have been reached */ static LocationAPI* createInstance(LocationCallbacks&); - /* destroy/cleans up the instance, which should be called when LocationAPI object is - no longer needed. LocationAPI* returned from createInstance will no longer valid - after destroy is called */ - void destroy(); + /* destroy/cleans up the instance, which should be called when LocationControlAPI object is + no longer needed. LocationControlAPI* returned from createInstance will no longer valid + after destroy is called. + If the caller allocates the memory for LocationControlCallbacks used in + LocationControlAPI::createInstance, then the caller must ensure that the memory still remains + valid until destroyCompleteCb is invoked. + */ + void destroy(locationApiDestroyCompleteCallback destroyCompleteCb=nullptr); /* updates/changes the callbacks that will be called. mandatory callbacks must be present for callbacks to be successfully updated diff --git a/location/LocationDataTypes.h b/location/LocationDataTypes.h index f35b2200..61598be8 100644 --- a/location/LocationDataTypes.h +++ b/location/LocationDataTypes.h @@ -1274,6 +1274,9 @@ typedef std::function locationSystemInfoCallback; +typedef std::function locationApiDestroyCompleteCallback; + typedef struct { size_t size; // set to sizeof(LocationCallbacks) capabilitiesCallback capabilitiesCb; // mandatory diff --git a/location/location_interface.h b/location/location_interface.h index e9e2fe89..480bbdb4 100644 --- a/location/location_interface.h +++ b/location/location_interface.h @@ -46,12 +46,14 @@ typedef std::function GnssEnergyConsumedCallback; +typedef void (*removeClientCompleteCallback)(LocationAPI* client); + struct GnssInterface { size_t size; void (*initialize)(void); void (*deinitialize)(void); void (*addClient)(LocationAPI* client, const LocationCallbacks& callbacks); - void (*removeClient)(LocationAPI* client); + void (*removeClient)(LocationAPI* client, removeClientCompleteCallback rmClientCb); void (*requestCapabilities)(LocationAPI* client); uint32_t (*startTracking)(LocationAPI* client, TrackingOptions&); void (*updateTrackingOptions)(LocationAPI* client, uint32_t id, TrackingOptions&); @@ -87,7 +89,7 @@ struct FlpInterface { void (*initialize)(void); void (*deinitialize)(void); void (*addClient)(LocationAPI* client, const LocationCallbacks& callbacks); - void (*removeClient)(LocationAPI* client); + void (*removeClient)(LocationAPI* client, removeClientCompleteCallback rmClientCb); void (*requestCapabilities)(LocationAPI* client); uint32_t (*startTracking)(LocationAPI* client, TrackingOptions&); void (*updateTrackingOptions)(LocationAPI* client, uint32_t id, TrackingOptions&); @@ -104,7 +106,7 @@ struct GeofenceInterface { void (*initialize)(void); void (*deinitialize)(void); void (*addClient)(LocationAPI* client, const LocationCallbacks& callbacks); - void (*removeClient)(LocationAPI* client); + void (*removeClient)(LocationAPI* client, removeClientCompleteCallback rmClientCb); void (*requestCapabilities)(LocationAPI* client); uint32_t* (*addGeofences)(LocationAPI* client, size_t count, GeofenceOption*, GeofenceInfo*); void (*removeGeofences)(LocationAPI* client, size_t count, uint32_t* ids);