Merge changes I896af608,I2e2a65bc,I8d9ae8b9,Ibb07666a into nyc-mr1-dev

* changes:
  ContextHubService: Fix app query
  ContextHubService: Don't double report app loads
  ContextHubService: Make app handle size match Java
  ContextHubService: Update caches on nanoapp unload

3 files changed, 80 insertions, 49 deletions

diff --git a/core/java/android/hardware/location/ContextHubService.java b/core/java/android/hardware/location/ContextHubService.java
index 062c9580c1e4..a699b687918b 100644
--- a/core/java/android/hardware/location/ContextHubService.java
+++ b/core/java/android/hardware/location/ContextHubService.java
@@ -322,9 +322,16 @@ public class ContextHubService extends IContextHubService.Stub {
         appInfo.setNeededReadMemBytes(PRE_LOADED_APP_MEM_REQ);
         appInfo.setNeededWriteMemBytes(PRE_LOADED_APP_MEM_REQ);
 
+        String action;
+        if (mNanoAppHash.containsKey(appInstanceHandle)) {
+            action = "Updated";
+        } else {
+            action = "Added";
+        }
+
         mNanoAppHash.put(appInstanceHandle, appInfo);
-        Log.d(TAG, "Added app instance " + appInstanceHandle + " with id " + appId
-              + " version " + appVersion);
+        Log.d(TAG, action + " app instance " + appInstanceHandle + " with id "
+              + appId + " version " + appVersion);
 
         return 0;
     }
diff --git a/core/java/android/hardware/location/NanoAppInstanceInfo.java b/core/java/android/hardware/location/NanoAppInstanceInfo.java
index 71a5a8816f58..ac6d83f622b2 100644
--- a/core/java/android/hardware/location/NanoAppInstanceInfo.java
+++ b/core/java/android/hardware/location/NanoAppInstanceInfo.java
@@ -113,7 +113,12 @@ public class NanoAppInstanceInfo {
     }
 
     /**
-     * Set the application version
+     * Get the application version
+     *
+     * NOTE: There is a race condition where shortly after loading, this
+     * may return -1 instead of the correct version.
+     *
+     * TODO(b/30970527): Fix this race condition.
      *
      * @return int - version of the app
      */
diff --git a/core/jni/android_hardware_location_ContextHubService.cpp b/core/jni/android_hardware_location_ContextHubService.cpp
index 9515a0e336aa..d0c66a38ae4f 100644
--- a/core/jni/android_hardware_location_ContextHubService.cpp
+++ b/core/jni/android_hardware_location_ContextHubService.cpp
@@ -34,11 +34,11 @@
 #include "JNIHelp.h"
 #include "core_jni_helpers.h"
 
-static constexpr int OS_APP_ID = -1;
+static constexpr jint OS_APP_ID = -1;
 static constexpr uint64_t ALL_APPS = UINT64_C(0xFFFFFFFFFFFFFFFF);
 
-static constexpr int MIN_APP_ID = 1;
-static constexpr int MAX_APP_ID = 128;
+static constexpr jint MIN_APP_ID = 1;
+static constexpr jint MAX_APP_ID = 128;
 
 static constexpr size_t MSG_HEADER_SIZE = 4;
 static constexpr size_t HEADER_FIELD_MSG_TYPE = 0;
@@ -102,7 +102,7 @@ struct context_hub_info_s {
 struct app_instance_info_s {
     uint64_t truncName;          // Possibly truncated name for logging
     uint32_t hubHandle;          // Id of the hub this app is on
-    int instanceId;              // system wide unique instance id - assigned
+    jint instanceId;             // system wide unique instance id - assigned
     struct hub_app_info appInfo; // returned from the HAL
 };
 
@@ -153,8 +153,8 @@ struct contextHubServiceDb_s {
     int initialized;
     context_hub_info_s hubInfo;
     jniInfo_s jniInfo;
-    std::queue<int> freeIds;
-    std::unordered_map<int, app_instance_info_s> appInstances;
+    std::queue<jint> freeIds;
+    std::unordered_map<jint, app_instance_info_s> appInstances;
     txnManager_s txnManager;
 };
 
@@ -259,16 +259,17 @@ static int get_hub_id_for_hub_handle(int hubHandle) {
     }
 }
 
-static int get_hub_handle_for_app_instance(int id) {
+static int get_hub_handle_for_app_instance(jint id) {
     if (!db.appInstances.count(id)) {
-        ALOGD("%s: Cannot find app for app instance %d", __FUNCTION__, id);
+        ALOGD("%s: Cannot find app for app instance %" PRId32,
+              __FUNCTION__, id);
         return -1;
     }
 
     return db.appInstances[id].hubHandle;
 }
 
-static int get_hub_id_for_app_instance(int id) {
+static int get_hub_id_for_app_instance(jint id) {
     int hubHandle = get_hub_handle_for_app_instance(id);
 
     if (hubHandle < 0) {
@@ -278,7 +279,7 @@ static int get_hub_id_for_app_instance(int id) {
     return db.hubInfo.hubs[hubHandle].hub_id;
 }
 
-static int get_app_instance_for_app_id(uint64_t app_id) {
+static jint get_app_instance_for_app_id(uint64_t app_id) {
     auto end = db.appInstances.end();
     for (auto current = db.appInstances.begin(); current != end; ++current) {
         if (current->second.appInfo.app_name.id == app_id) {
@@ -289,9 +290,10 @@ static int get_app_instance_for_app_id(uint64_t app_id) {
     return -1;
 }
 
-static int set_dest_app(hub_message_t *msg, int id) {
+static int set_dest_app(hub_message_t *msg, jint id) {
     if (!db.appInstances.count(id)) {
-        ALOGD("%s: Cannot find app for app instance %d", __FUNCTION__, id);
+        ALOGD("%s: Cannot find app for app instance %" PRId32,
+              __FUNCTION__, id);
         return -1;
     }
 
@@ -303,7 +305,7 @@ static void query_hub_for_apps(uint64_t appId, uint32_t hubHandle) {
     hub_message_t msg;
     query_apps_request_t queryMsg;
 
-    queryMsg.app_name.id = NANOAPP_VENDOR_ALL_APPS;
+    queryMsg.app_name.id = appId;
 
     msg.message_type = CONTEXT_HUB_QUERY_APPS;
     msg.message_len  = sizeof(queryMsg);
@@ -322,7 +324,7 @@ static void sendQueryForApps(uint64_t appId) {
     }
 }
 
-static int return_id(int id) {
+static int return_id(jint id) {
     // Note : This method is not thread safe.
     // id returned is guaranteed to be in use
     if (id >= 0) {
@@ -333,9 +335,9 @@ static int return_id(int id) {
     return -1;
 }
 
-static int generate_id() {
+static jint generate_id() {
     // Note : This method is not thread safe.
-    int retVal = -1;
+    jint retVal = -1;
 
     if (!db.freeIds.empty()) {
         retVal = db.freeIds.front();
@@ -346,8 +348,8 @@ static int generate_id() {
 }
 
 
-static int add_app_instance(const hub_app_info *appInfo, uint32_t hubHandle,
-        int appInstanceHandle, JNIEnv *env) {
+static jint add_app_instance(const hub_app_info *appInfo, uint32_t hubHandle,
+        jint appInstanceHandle, JNIEnv *env) {
 
     ALOGI("Loading App");
 
@@ -355,10 +357,12 @@ static int add_app_instance(const hub_app_info *appInfo, uint32_t hubHandle,
     app_instance_info_s entry;
     assert(appInfo);
 
+    const char *action = "Updated";
     if (db.appInstances.count(appInstanceHandle) == 0) {
+        action = "Added";
         appInstanceHandle = generate_id();
         if (appInstanceHandle < 0) {
-            ALOGE("Cannot find resources to add app instance %d",
+            ALOGE("Cannot find resources to add app instance %" PRId32,
                   appInstanceHandle);
             return -1;
         }
@@ -378,36 +382,42 @@ static int add_app_instance(const hub_app_info *appInfo, uint32_t hubHandle,
                        hubHandle, entry.instanceId, entry.truncName,
                        entry.appInfo.version);
 
-    ALOGW("Added App 0x%" PRIx64 " on hub Handle %" PRId32
-          " as appInstance %d", entry.truncName,
+    ALOGW("%s App 0x%" PRIx64 " on hub Handle %" PRId32
+          " as appInstance %" PRId32, action, entry.truncName,
           entry.hubHandle, appInstanceHandle);
 
     return appInstanceHandle;
 }
 
-int delete_app_instance(int id, JNIEnv *env) {
-    if (!db.appInstances.count(id)) {
-        ALOGW("Cannot find App id : %d", id);
-        return -1;
-    }
+int delete_app_instance(jint id, JNIEnv *env) {
+    bool fullyDeleted = true;
 
+    if (db.appInstances.count(id)) {
+        db.appInstances.erase(id);
+    } else {
+        ALOGW("Cannot delete App id (%" PRId32 ") from the JNI C++ cache", id);
+        fullyDeleted = false;
+    }
     return_id(id);
-    db.appInstances.erase(id);
-    if (env->CallIntMethod(db.jniInfo.jContextHubService,
+
+    if ((env == nullptr) ||
+        (env->CallIntMethod(db.jniInfo.jContextHubService,
                        db.jniInfo.contextHubServiceDeleteAppInstance,
-                       id) != 0) {
-        ALOGW("Could not delete App id : %d", id);
-        return -1;
+                       id) != 0)) {
+        ALOGW("Cannot delete App id (%" PRId32 ") from Java cache", id);
+        fullyDeleted = false;
     }
 
-    ALOGI("Deleted App id : %d", id);
-
-    return 0;
+    if (fullyDeleted) {
+        ALOGI("Deleted App id : %" PRId32, id);
+        return 0;
+    }
+    return -1;
 }
 
 static int startLoadAppTxn(uint64_t appId, int hubHandle) {
     app_instance_info_s *txnInfo = (app_instance_info_s *)malloc(sizeof(app_instance_info_s));
-    int instanceId = generate_id();
+    jint instanceId = generate_id();
 
     if (!txnInfo || instanceId < 0) {
         return_id(instanceId);
@@ -432,8 +442,8 @@ static int startLoadAppTxn(uint64_t appId, int hubHandle) {
     return 0;
 }
 
-static int startUnloadAppTxn(uint32_t appInstanceHandle) {
-    uint32_t *txnData = (uint32_t *) malloc(sizeof(uint32_t));
+static int startUnloadAppTxn(jint appInstanceHandle) {
+    jint *txnData = (jint *) malloc(sizeof(jint));
     if (!txnData) {
         ALOGW("Cannot allocate memory to start unload transaction");
         return -1;
@@ -454,7 +464,6 @@ static void initContextHubService() {
     int err = 0;
     db.hubInfo.hubs = nullptr;
     db.hubInfo.numHubs = 0;
-    int i;
 
     err = hw_get_module(CONTEXT_HUB_MODULE_ID,
                         (hw_module_t const**)(&db.hubInfo.contextHubModule));
@@ -465,7 +474,7 @@ static void initContextHubService() {
     }
 
     // Prep for storing app info
-    for(i = MIN_APP_ID; i <= MAX_APP_ID; i++) {
+    for (jint i = MIN_APP_ID; i <= MAX_APP_ID; i++) {
         db.freeIds.push(i);
     }
 
@@ -485,7 +494,7 @@ static void initContextHubService() {
                 return;
             }
 
-            for (i = 0; i < db.hubInfo.numHubs; i++) {
+            for (int i = 0; i < db.hubInfo.numHubs; i++) {
                 db.hubInfo.cookies[i] = db.hubInfo.hubs[i].hub_id;
                 ALOGI("Subscribing to hubHandle %d with OS App name %" PRIu64, i, db.hubInfo.hubs[i].os_app_name.id);
                 if (db.hubInfo.contextHubModule->subscribe_messages(db.hubInfo.hubs[i].hub_id,
@@ -547,13 +556,15 @@ int handle_query_apps_response(const uint8_t *msg, int msgLen,
         memcpy(&info, unalignedInfoAddr, sizeof(info));
         // We will only have one instance of the app
         // TODO : Change this logic once we support multiple instances of the same app
-        int appInstance = get_app_instance_for_app_id(info.app_name.id);
+        jint appInstance = get_app_instance_for_app_id(info.app_name.id);
         add_app_instance(&info, hubHandle, appInstance, env);
     }
 
     return 0;
 }
 
+// TODO(b/30807327): Do not use raw bytes for additional data.  Use the
+//     JNI interfaces for the appropriate types.
 static void passOnOsResponse(uint32_t hubHandle, uint32_t msgType,
                              status_response_t *rsp, int8_t *additionalData,
                              size_t additionalDataLen) {
@@ -613,7 +624,13 @@ void closeUnloadTxn(bool success) {
 
     if (success && fetchTxnData(&txnId, &txnData) == 0 &&
         txnId == CONTEXT_HUB_UNLOAD_APP) {
-        db.appInstances.erase(*(uint32_t *)txnData);
+        JNIEnv *env;
+        if ((db.jniInfo.vm)->AttachCurrentThread(&env, nullptr) != JNI_OK) {
+            ALOGW("Could not attach to JVM !");
+            env = nullptr;
+        }
+        jint handle = *reinterpret_cast<jint *>(txnData);
+        delete_app_instance(handle, env);
     } else {
         ALOGW("Could not unload the app successfully ! success %d, txnData %p", success, txnData);
     }
@@ -621,7 +638,7 @@ void closeUnloadTxn(bool success) {
     closeTxn();
 }
 
-void closeLoadTxn(bool success, int *appInstanceHandle) {
+void closeLoadTxn(bool success, jint *appInstanceHandle) {
     void *txnData;
     hub_messages_e txnId;
 
@@ -668,6 +685,7 @@ static void invalidateNanoApps(uint32_t hubHandle) {
 
     if ((db.jniInfo.vm)->AttachCurrentThread(&env, nullptr) != JNI_OK) {
         ALOGW("Could not attach to JVM !");
+        env = nullptr;
     }
 
     auto end = db.appInstances.end();
@@ -697,7 +715,7 @@ static int handle_os_message(uint32_t msgType, uint32_t hubHandle,
       case CONTEXT_HUB_UNLOAD_APP:
           if (isValidOsStatus(msg, msgLen, &rsp)) {
               if (msgType == CONTEXT_HUB_LOAD_APP) {
-                  int appInstanceHandle;
+                  jint appInstanceHandle;
                   closeLoadTxn(rsp.result == 0, &appInstanceHandle);
                   passOnOsResponse(hubHandle, msgType, &rsp, (int8_t *)(&appInstanceHandle),
                                    sizeof(appInstanceHandle));
@@ -778,7 +796,7 @@ int context_hub_callback(uint32_t hubId,
     if (messageType < CONTEXT_HUB_TYPE_PRIVATE_MSG_BASE) {
         handle_os_message(messageType, hubHandle, (uint8_t*) msg->message, msg->message_len);
     } else {
-        int appHandle = get_app_instance_for_app_id(msg->app_name.id);
+        jint appHandle = get_app_instance_for_app_id(msg->app_name.id);
         if (appHandle < 0) {
             ALOGE("Filtering out message due to invalid App Instance.");
         } else {
@@ -1051,7 +1069,8 @@ static jint nativeSendMessage(JNIEnv *env, jobject instance, jintArray header_,
         ALOGD("Asking HAL to remove app");
         retVal = db.hubInfo.contextHubModule->send_message(hubId, &msg);
     } else {
-      ALOGD("Could not find app instance %d on hubHandle %d, setAddress %d",
+      ALOGD("Could not find app instance %" PRId32 " on hubHandle %" PRId32
+            ", setAddress %d",
             header[HEADER_FIELD_APP_INSTANCE],
             header[HEADER_FIELD_HUB_HANDLE],
             (int)setAddressSuccess);