From 66d1ba22264303e705bbe126578c0c99cdb80dba Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Tue, 11 May 2021 20:00:43 +0300 Subject: [PATCH] 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 Suggested-by: Scott Murray Change-Id: I97776938e2d0971eba509c1ea36462899d053a24 --- src/hs-clientmanager.cpp | 53 +++++++++++++++++++----------------------------- 1 file 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 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; -- 2.16.6