Fix business logic in slp-pkgmgr-server. 33/30833/2 accepted/tizen/common/20141125.180004 accepted/tizen/ivi/20141203.010012 submit/tizen_common/20141125.175019 submit/tizen_ivi/20141201.000000 submit/tizen_ivi/20141201.014922 submit/tizen_ivi/20141202.062010
authorBaptiste DURAND <baptiste.durand@open.eurogiciel.org>
Tue, 25 Nov 2014 17:19:00 +0000 (18:19 +0100)
committerBaptiste DURAND <baptiste.durand@open.eurogiciel.org>
Tue, 25 Nov 2014 17:33:34 +0000 (18:33 +0100)
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 <baptiste.durand@open.eurogiciel.org>
server/src/pkgmgr-server.c
server/src/pm-queue.c

index 6e5d3a0b424dd8b5a9fe7bb39dd1464dbc0c070e..8084ebc6edea48332f55702def06ec0ff1c6ea08 100755 (executable)
@@ -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;
                }
index 6ac727b91c7a0a22b7972a811727c6545e83e84a..3880fd57707a6131036ed33bfa9b0b37dc4643d4 100755 (executable)
@@ -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;
 }