From c5abded9a1d299e11251de72d1267f5b18d44020 Mon Sep 17 00:00:00 2001 From: Baptiste DURAND Date: Tue, 25 Nov 2014 18:19:00 +0100 Subject: [PATCH] Fix business logic in slp-pkgmgr-server. The current logic of pkgmgr-server is not robust to a burst of installation request. This leads to have a deadlock into pkgmgr-server. And it doens't answer to reuqest anymore Force to wait backend end before returning for each job that is dependent of a backend Make pm_queue thread safe. (we can have multiple request that leads to have concurencial meassge push ) BUG-Tizen=TC-2127 Change-Id: Ib3b3b4c4c9043b395869f0ee6c16da6db0da932d Signed-off-by: Baptiste DURAND --- server/src/pkgmgr-server.c | 85 ++++++++++++++++++++++++++++++++++++++-------- server/src/pm-queue.c | 13 +++++-- 2 files changed, 81 insertions(+), 17 deletions(-) diff --git a/server/src/pkgmgr-server.c b/server/src/pkgmgr-server.c index 6e5d3a0..8084ebc 100755 --- a/server/src/pkgmgr-server.c +++ b/server/src/pkgmgr-server.c @@ -175,6 +175,7 @@ static void response_cb1(void *data, void *event_info); static void response_cb2(void *data, void *event_info); static int create_popup(struct appdata *ad); static void sighandler(int signo); +static void _wait_backend(pid_t pid); static int __get_position_from_pkg_type(char *pkgtype); static int __is_efl_tpk_app(char *pkgpath); static int __xsystem(const char *argv[]); @@ -864,6 +865,53 @@ gboolean send_fail_signal(void *data) return FALSE; } +static void _wait_backend(pid_t pid) +{ + int status; + pid_t cpid; + int i = 0; + backend_info *ptr = NULL; + ptr = begin; + cpid = waitpid(pid, &status, 0); + if (cpid != pid) + ERR("WaitBackend Faillure %d",cpid); + else if (WIFEXITED(status)) { + DBG("child NORMAL exit [%d]\n", cpid); + for(i = 0; i < num_of_backends; i++) + { + if (cpid == (ptr + i)->pid) { + __set_backend_free(i); + __set_backend_mode(i); + __unset_recovery_mode((ptr + i)->pkgid, (ptr + i)->pkgtype); + DBG("clear the status of [%d] nb %d \n", cpid, i); + + break; + } + else + DBG("PID of registered backend is [%d]\n", (ptr + i)->pid); + + } + } + else if (WIFSIGNALED(status)) { + DBG("child SIGNALED exit [%d]\n", cpid); + /*get the pkgid and pkgtype to send fail signal*/ + for(i = 0; i < num_of_backends; i++) + { + if (cpid == (ptr + i)->pid) { + __set_backend_free(i); + __set_backend_mode(i); + __unset_recovery_mode((ptr + i)->pkgid, (ptr + i)->pkgtype); + strncpy(pname, (ptr + i)->pkgid, MAX_PKG_NAME_LEN-1); + strncpy(ptype, (ptr + i)->pkgtype, MAX_PKG_TYPE_LEN-1); + strncpy(args, (ptr + i)->args, MAX_PKG_ARGS_LEN-1); + g_idle_add(send_fail_signal, NULL); + break; + } + } + } +} + + static void sighandler(int signo) { int status; @@ -882,8 +930,13 @@ static void sighandler(int signo) __set_backend_free(i); __set_backend_mode(i); __unset_recovery_mode((ptr + i)->pkgid, (ptr + i)->pkgtype); + DBG("clear the status of [%d] nb %d \n", cpid, i); + break; } + else + DBG("PID of registered backend is [%d]\n", (ptr + i)->pid); + } } else if (WIFSIGNALED(status)) { @@ -1008,11 +1061,6 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, else if (strstr(args, " -d ") || strstr(args, " '-d' ")) { ad->op_type = OPERATION_UNINSTALL; - - /* 2011-04-01 - Change the mode temporarily. This should be removed */ - /*strncat(item->args, " -q", - strlen(" -q"));*/ } else if (strstr(args, " -r ") || strstr(args, " '-r' ")) ad->op_type = OPERATION_REINSTALL; @@ -1023,7 +1071,6 @@ void req_cb(void *cb_data, uid_t uid, const char *req_id, const int req_type, if (err != 0) { *ret = COMM_RET_ERROR; DBG("create popup failed\n"); - /*queue_job(NULL);*/ goto err; } else { *ret = COMM_RET_OK; @@ -1482,16 +1529,18 @@ gboolean queue_job(void *data) 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; - } + 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; char *backend_cmd = NULL; @@ -1500,7 +1549,8 @@ pop: if ( (item == NULL) || (item->req_type == -1) ) { if(item) free(item); - goto pop; + DBG("the queue is empty"); + return FALSE; } __set_backend_busy((pos + num_of_backends - 1) % num_of_backends); __set_recovery_mode(item->pkgid, item->pkg_type); @@ -1614,6 +1664,7 @@ pop: default: /* parent */ DBG("parent \n"); + _wait_backend((ptr + x)->pid); _save_queue_status(item, "done"); break; } @@ -1778,6 +1829,7 @@ pop: default: /* parent */ DBG("parent exit\n"); + _wait_backend((ptr + x)->pid); _save_queue_status(item, "done"); break; } @@ -1882,6 +1934,7 @@ pop: default: /* parent */ DBG("parent \n"); + _wait_backend((ptr + x)->pid); _save_queue_status(item, "done"); break; } @@ -1986,6 +2039,7 @@ pop: default: /* parent */ DBG("parent \n"); + _wait_backend((ptr + x)->pid); _save_queue_status(item, "done"); break; } @@ -2044,6 +2098,7 @@ pop: default: /* parent */ DBG("parent exit\n"); + _wait_backend((ptr + x)->pid); _save_queue_status(item, "done"); break; } diff --git a/server/src/pm-queue.c b/server/src/pm-queue.c index 6ac727b..3880fd5 100755 --- a/server/src/pm-queue.c +++ b/server/src/pm-queue.c @@ -43,6 +43,7 @@ static int __entry_exist(char *backend); static int __is_pkg_supported(char *pkgtype); queue_info_map *start = NULL; +pthread_mutex_t pm_mutex; int entries = 0; int slot = 0; int num_of_backends = 0; @@ -243,6 +244,8 @@ int _pm_queue_init() } free(namelist); num_of_backends = slot; + pthread_mutex_init(&pm_mutex, NULL); + #ifdef DEBUG_INFO /*Debug info*/ printf("Queue Info Map\n"); @@ -256,6 +259,7 @@ int _pm_queue_init() ptr++; } #endif + return 0; } @@ -269,12 +273,14 @@ int _pm_queue_push(pm_dbus_msg *item) if (ret == 0) return -1; + pthread_mutex_lock(&pm_mutex); cur = __get_head_from_pkgtype(item); tmp = cur; data = _add_node(); if (!data) { /* fail to allocate mem */ fprintf(stderr, "Fail to allocate memory\n"); + pthread_mutex_unlock(&pm_mutex); return -1; } @@ -299,6 +305,7 @@ int _pm_queue_push(pm_dbus_msg *item) tmp->next = data; } + pthread_mutex_unlock(&pm_mutex); return 0; } @@ -308,7 +315,7 @@ pm_dbus_msg *_pm_queue_pop(int position) pm_dbus_msg *ret; pm_queue_data *cur = NULL; pm_queue_data *saveptr = NULL; - queue_info_map *ptr = start; + queue_info_map *ptr = NULL; int i = 0; ret = (pm_dbus_msg *) malloc(sizeof(pm_dbus_msg)); @@ -317,7 +324,8 @@ pm_dbus_msg *_pm_queue_pop(int position) return NULL; } memset(ret, 0x00, sizeof(pm_dbus_msg)); - + pthread_mutex_lock(&pm_mutex); + ptr = start; for(i = 0; i < entries; i++) { if (ptr->queue_slot == position) { @@ -354,6 +362,7 @@ pm_dbus_msg *_pm_queue_pop(int position) } ptr++; } + pthread_mutex_unlock(&pm_mutex); return ret; } -- 2.7.4