clientmanager: Do not check for running applicationsmarlin_12.90.0marlin/12.90.0lamprey_11.92.0lamprey/11.92.012.90.011.92.0

Due to some unforeseen consequences we can't really check for running
applications, in an attempt to start them if they died/or have killed
(either legitimate or not). To avoid deadlocking just have another pass
over the clients and document the fact that requires a bit more digging
to fix up correctly.

Bug-AGL: SPEC-3902

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Scott Murray <scott.murray@konsulko.com>
Change-Id: I97776938e2d0971eba509c1ea36462899d053a24

1 files changed, 21 insertions, 32 deletions

diff --git a/src/hs-clientmanager.cpp b/src/hs-clientmanager.cpp
index 66fd699..7d1407e 100644
--- a/src/hs-clientmanager.cpp
+++ b/src/hs-clientmanager.cpp
@@ -161,43 +161,32 @@ void HS_ClientManager::removeClientCtxt(void *data)
 }
 
 static int
-is_application_running(afb_req_t request, std::string id)
+is_application_running(std::string appid, std::unordered_map<std::string, HS_Client*> client_list)
 {
     bool app_still_running = false;
-    struct json_object *jobj = nullptr;
 
-    HS_AfmMainProxy afm_proxy;
+    std::string id(appid);
+    auto ip = client_list.find(id);
 
-    // note this is sync, so this might block if afm-system-daemon is down
-    afm_proxy.ps(request->api, &jobj);
-
-    if (jobj) {
-	    size_t len = json_object_array_length(jobj);
-	    for (size_t i = 0; i < len; i++) {
-		struct json_object *aid;
-		struct json_object *item =
-		    json_object_array_get_idx(jobj, i);
-
-		bool isFound = json_object_object_get_ex(item, "id", &aid);
-		if (isFound) {
-		    const char *str_appid = json_object_get_string(aid);
-		    if (strcmp(str_appid, id.c_str()) == 0) {
-			app_still_running = true;
-			break;
-		    }
-		}
-	    }
+    // this will always be case as the removeClientCtxt() is never called as
+    // clients do not handle the subscribe at all at this point. Not only that
+    // but is redudant but we keep at as it to highlight the fact that we're
+    // missing a feature to check if applications died or not (legitimate or not).
+    //
+    // Using ps (HS_AfmMainProxy::ps -- a sync version of checking if running
+    // applications) seem to block in afm-system-daemon (see SPEC-3902), and
+    // doing w/ it with an async version of ps doesn't work because we don't
+    // have a valid clientCtx (required, and only possible if the application
+    // subscribed themselves, which no longer happens)
+    //
+    // FIXME: We need another way of handling this would be necessary to correctly
+    // handle the case where the app died.
+    if (ip != client_list.end()) {
+        app_still_running = true;
     }
 
-    if (!app_still_running) {
-        // we don't remove it from the context list as we're haven't really subscribed,
-        // and we just need to remove it from client_list, which happens here. We also
-	// return AFB_REQ_NOT_STARTED_APPLICATION which will attempt to start it (again).
-        HS_ClientManager::instance()->removeClient(id);
-        return AFB_REQ_NOT_STARTED_APPLICATION;
-    }
-
-    return 0;
+    // > 0 means err
+    return app_still_running != true;
 }
 
 /**
@@ -234,7 +223,7 @@ int HS_ClientManager::handleRequest(afb_req_t request, const char *verb, const c
 	    // automatically removes the application from client_list.
 	    // That is exactly how "subscribe" verb is handled below.
             if (strcasecmp(verb, "showWindow") == 0) {
-                ret = is_application_running(request, id);
+                ret = is_application_running(id, client_list);
                 if (ret == AFB_REQ_NOT_STARTED_APPLICATION) {
                     AFB_INFO("%s is not running. Will attempt to start it", appid);
                     return ret;