From cc62763ac7ffa287b0e5b22de3d2e678f3545504 Mon Sep 17 00:00:00 2001 From: Sangyoon Jang Date: Mon, 2 Feb 2015 11:29:35 +0900 Subject: [PATCH] Fix for potential case of falling into infinite loop when pop queue TC-2402 if backend's process takes very long times, the client who requests something to server will get timeout because the server cannot receive the request from client because it is busy for looping on queue_job when received dbus request, add idler to handle it at the next event loop iteration (replace direct call of queue_job with add an idler) and add again when backend exit to handle next queued request remove unwanted local variable initializing Change-Id: I854b0b90356d9d875951b053e2c090eae1192a6b Signed-off-by: Sangyoon Jang --- server/src/pkgmgr-server.c | 136 +++++++++++++++++++++++---------------------- 1 file changed, 69 insertions(+), 67 deletions(-) diff --git a/server/src/pkgmgr-server.c b/server/src/pkgmgr-server.c index e931800..d587c25 100644 --- a/server/src/pkgmgr-server.c +++ b/server/src/pkgmgr-server.c @@ -428,8 +428,8 @@ void response_cb1(void *data, void *event_info) /* Free resource */ free(ad->item); /***************/ - if (ret == 0) - g_idle_add(queue_job, NULL); + if (ret) + ERR("failed to push queue item"); DBG("end of response_cb()\n"); @@ -752,6 +752,8 @@ static gboolean pipe_io_handler(GIOChannel *io, GIOCondition cond, gpointer data DBG("backend[%s] exit", ptr->pkgtype); } + g_idle_add(queue_job, NULL); + return TRUE; } @@ -816,8 +818,7 @@ static int __register_signal_handler(void) return -1; } - if (g_timeout_add_seconds(2, exit_server, NULL)) - DBG("g_timeout_add_seconds() Added to Main Loop"); + g_timeout_add_seconds(2, exit_server, NULL); sig_reg = 1; return 0; @@ -827,9 +828,8 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, const char *pkg_type, const char *pkgid, const char *args, const char *cookie, int *ret) { - int err = -1; - int p = 0; - int cookie_result = 0; + int p; + int cookie_result; DBG(">> in callback >> Got request: [%s] [%d] [%s] [%s] [%s] [%s]", req_id, req_type, pkg_type, pkgid, args, cookie); @@ -868,6 +868,7 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, *ret = COMM_RET_ERROR; goto err; } + g_idle_add(queue_job, NULL); DBG("req_type=(%d) drawing_popup=(%d) backend_flag=(%d)\n", req_type, drawing_popup, backend_flag); @@ -890,13 +891,15 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, ((quiet = strstr(args, " '-q'")) && (quiet[strlen(quiet)] == '\0'))) { /* quiet mode */ - err = _pm_queue_push(item); + if (_pm_queue_push(item)) { + ERR("failed to push queue item"); + *ret = COMM_RET_ERROR; + goto err; + } p = __get_position_from_pkg_type(item->pkg_type); __set_backend_mode(p); /* Free resource */ free(item); - if (err == 0) - g_idle_add(queue_job, NULL); *ret = COMM_RET_OK; } else { /* non quiet mode */ @@ -919,8 +922,7 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, else ad->op_type = OPERATION_MAX; - err = create_popup(ad); - if (err != 0) { + if (create_popup(ad)) { *ret = COMM_RET_ERROR; DBG("create popup failed\n"); goto err; @@ -938,20 +940,25 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, break; case COMM_REQ_TO_ACTIVATOR: /* In case of activate, there is no popup */ - err = _pm_queue_push(item); + if (_pm_queue_push(item)) { + ERR("failed to push queue item"); + *ret = COMM_RET_ERROR; + goto err; + } p = __get_position_from_pkg_type(item->pkg_type); __set_backend_mode(p); /* Free resource */ free(item); -/* g_idle_add(queue_job, NULL); */ - if (err == 0) - queue_job(NULL); *ret = COMM_RET_OK; break; case COMM_REQ_TO_CLEARER: /* In case of clearer, there is no popup */ - err = _pm_queue_push(item); + if (_pm_queue_push(item)) { + ERR("failed to push queue item"); + *ret = COMM_RET_ERROR; + goto err; + } p = __get_position_from_pkg_type(item->pkg_type); /*the backend shows the success/failure popup so this request is non quiet*/ @@ -959,9 +966,6 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, /* Free resource */ free(item); -/* g_idle_add(queue_job, NULL); */ - if (err == 0) - queue_job(NULL); *ret = COMM_RET_OK; break; case COMM_REQ_TO_MOVER: @@ -974,15 +978,17 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, } /* In case of mover, there is no popup */ - err = _pm_queue_push(item); + if (_pm_queue_push(item)) { + ERR("failed to push queue item"); + *ret = COMM_RET_ERROR; + goto err; + } p = __get_position_from_pkg_type(item->pkg_type); /*the backend shows the success/failure popup so this request is non quiet*/ __unset_backend_mode(p); /* Free resource */ free(item); - if (err == 0) - queue_job(NULL); *ret = COMM_RET_OK; break; case COMM_REQ_CANCEL: @@ -1002,28 +1008,31 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, goto err; } - err = _pm_queue_push(item); + if (_pm_queue_push(item)) { + ERR("failed to push queue item"); + *ret = COMM_RET_ERROR; + goto err; + } p = __get_position_from_pkg_type(item->pkg_type); __set_backend_mode(p); /* Free resource */ free(item); - if (err == 0) - g_idle_add(queue_job, NULL); *ret = COMM_RET_OK; break; case COMM_REQ_CHECK_APP: case COMM_REQ_KILL_APP: /* In case of activate, there is no popup */ - err = _pm_queue_push(item); + if (_pm_queue_push(item)) { + ERR("failed to push queue item"); + *ret = COMM_RET_ERROR; + goto err; + } p = __get_position_from_pkg_type(item->pkg_type); __set_backend_mode(p); /* Free resource */ free(item); -/* g_idle_add(queue_job, NULL); */ - if (err == 0) - queue_job(NULL); *ret = COMM_RET_OK; break; case COMM_REQ_CLEAR_CACHE_DIR: @@ -1035,12 +1044,14 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, goto err; } - err = _pm_queue_push(item); + if (_pm_queue_push(item)) { + ERR("failed to push queue item"); + *ret = COMM_RET_ERROR; + goto err; + } p = __get_position_from_pkg_type(item->pkg_type); __set_backend_mode(p); - if (err == 0) - g_idle_add(queue_job, NULL); *ret = PKGMGR_R_OK; break; @@ -1467,50 +1478,41 @@ static void __exec_with_arg_vector(const char *cmd, char **argv, uid_t uid) gboolean queue_job(void *data) { - /* DBG("queue_job start"); */ - pm_dbus_msg *item; - backend_info *ptr = NULL; - ptr = begin; - int x = 0; - int pos = 0; - /* Pop a job from queue */ -pop: - if (!__is_backend_busy(pos % num_of_backends)) { - item = _pm_queue_pop(pos % num_of_backends); - pos = (pos + 1) % num_of_backends; - } else { - pos = (pos + 1) % num_of_backends; - goto pop; - } - int ret = 0; + pm_dbus_msg *item = NULL; + backend_info *ptr; + int x; + int ret; char *backend_cmd = NULL; - /* queue is empty and backend process is not running */ - if ( (item == NULL) || (item->req_type == -1) ) { - if(item) - free(item); - DBG("the queue is empty for backend %d ", (pos + num_of_backends - 1) % num_of_backends); + /* Pop a job from queue */ + for (x = 0, ptr = begin; x < num_of_backends; x++, ptr++) { + if (__is_backend_busy(x)) + continue; - if (pos == 0) // all backend messages queue are empty - return FALSE; - else // check the next backend message queue - goto pop; + item = _pm_queue_pop(x); + if (item && item->req_type != -1) + break; + free(item); } - __set_backend_busy((pos + num_of_backends - 1) % num_of_backends); + + /* all backend messages queue are empty or busy */ + if (x == num_of_backends) + return FALSE; + + __set_backend_busy(x); __set_recovery_mode(item->pkgid, item->pkg_type); /* fork */ _save_queue_status(item, "processing"); DBG("saved queue status. Now try fork()"); /*save pkg type and pkg name for future*/ - x = (pos + num_of_backends - 1) % num_of_backends; - strncpy((ptr + x)->pkgtype, item->pkg_type, MAX_PKG_TYPE_LEN-1); - strncpy((ptr + x)->pkgid, item->pkgid, MAX_PKG_NAME_LEN-1); - strncpy((ptr + x)->args, item->args, MAX_PKG_ARGS_LEN-1); - (ptr + x)->pid = fork(); - DBG("child forked [%d] for request type [%d]", (ptr + x)->pid, item->req_type); - - switch ((ptr + x)->pid) { + strncpy(ptr->pkgtype, item->pkg_type, MAX_PKG_TYPE_LEN-1); + strncpy(ptr->pkgid, item->pkgid, MAX_PKG_NAME_LEN-1); + strncpy(ptr->args, item->args, MAX_PKG_ARGS_LEN-1); + ptr->pid = fork(); + DBG("child forked [%d] for request type [%d]", ptr->pid, item->req_type); + + switch (ptr->pid) { case 0: /* child */ switch (item->req_type) { case COMM_REQ_TO_INSTALLER: -- 2.7.4