clientmanager: Do not check for running applications

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

diff --git a/src/hs-clientmanager.cpp b/src/hs-clientmanager.cpp
index 66fd699..7d1407e 100644 (file)
--- 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;