]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
Merged revisions 276074 via svnmerge from
authorJeff Peeler <jpeeler@digium.com>
Tue, 13 Jul 2010 19:01:29 +0000 (19:01 +0000)
committerJeff Peeler <jpeeler@digium.com>
Tue, 13 Jul 2010 19:01:29 +0000 (19:01 +0000)
https://origsvn.digium.com/svn/asterisk/trunk

................
  r276074 | jpeeler | 2010-07-13 12:37:40 -0500 (Tue, 13 Jul 2010) | 19 lines

  Merged revisions 275773 via svnmerge from
  https://origsvn.digium.com/svn/asterisk/branches/1.4

  ........
    r275773 | jpeeler | 2010-07-12 15:34:51 -0500 (Mon, 12 Jul 2010) | 12 lines

    Make user removals and traversals thread safe in meetme.

    Race conditions present in meetme involving the user list where a lack of
    locking has the potential for a user to be removed during a traversal or as in
    the case of the reporter after checking if the list is empty could cause a
    crash. Fixing this was done by convering the userlist to an ao2 container.

    (closes issue #17390)
    Reported by: Vince

    Review: https://reviewboard.asterisk.org/r/746/
  ........
................

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@276121 65c4cc65-6c06-0410-ace0-fbb531ad65f3

apps/app_meetme.c

index c42d19d2232f5965b0b51b2a9300ada03db72477..e53bf2997dc4f759359e4551b26f644ec2364d95 100644 (file)
@@ -636,7 +636,7 @@ struct ast_conference {
        struct ast_frame *transframe[32];
        struct ast_frame *origframe;
        struct ast_trans_pvt *transpath[32];
-       AST_LIST_HEAD_NOLOCK(, ast_conf_user) userlist;
+       struct ao2_container *usercontainer;
        AST_LIST_ENTRY(ast_conference) list;
        /* announce_thread related data */
        pthread_t announcethread;
@@ -1044,6 +1044,30 @@ static void conf_play(struct ast_channel *chan, struct ast_conference *conf, enu
                ast_autoservice_stop(chan);
 }
 
+static int user_no_cmp(void *obj, void *arg, int flags)
+{
+       struct ast_conf_user *user = obj;
+       int *user_no = arg;
+
+       if (user->user_no == *user_no) {
+               return (CMP_MATCH | CMP_STOP);
+       }
+
+       return 0;
+}
+
+static int user_max_cmp(void *obj, void *arg, int flags)
+{
+       struct ast_conf_user *user = obj;
+       int *max_no = arg;
+
+       if (user->user_no > *max_no) {
+               *max_no = user->user_no;
+       }
+
+       return 0;
+}
+
 /*!
  * \brief Find or create a conference
  *
@@ -1075,8 +1099,10 @@ static struct ast_conference *build_conf(char *confno, char *pin, char *pinadmin
                goto cnfout;
 
        /* Make a new one */
-       if (!(cnf = ast_calloc(1, sizeof(*cnf))))
+       if (!(cnf = ast_calloc(1, sizeof(*cnf))) ||
+               !(cnf->usercontainer = ao2_container_alloc(1, NULL, user_no_cmp))) {
                goto cnfout;
+       }
 
        ast_mutex_init(&cnf->playlock);
        ast_mutex_init(&cnf->listenlock);
@@ -1191,15 +1217,21 @@ static char *complete_meetmecmd(const char *line, const char *word, int pos, int
                        }
 
                        if (cnf) {
+                               struct ao2_iterator user_iter;
+                               user_iter = ao2_iterator_init(cnf->usercontainer, 0);
                                /* Search for the user */
-                               AST_LIST_TRAVERSE(&cnf->userlist, usr, list) {
+                               while((usr = ao2_iterator_next(&user_iter))) {
                                        snprintf(usrno, sizeof(usrno), "%d", usr->user_no);
-                                       if (!strncasecmp(word, usrno, len) && ++which > state)
+                                       if (!strncasecmp(word, usrno, len) && ++which > state) {
+                                               ao2_ref(usr, -1);
                                                break;
+                                       }
+                                       ao2_ref(usr, -1);
                                }
+                               ao2_iterator_destroy(&user_iter);
+                               AST_LIST_UNLOCK(&confs);
+                               return usr ? ast_strdup(usrno) : NULL;
                        }
-                       AST_LIST_UNLOCK(&confs);
-                       return usr ? ast_strdup(usrno) : NULL;
                }
        }
 
@@ -1286,6 +1318,7 @@ static char *meetme_show_cmd(struct ast_cli_entry *e, int cmd, struct ast_cli_ar
                ast_free(cmdline);
                return CLI_SUCCESS;
        } else if (strcmp(a->argv[1], "list") == 0) {
+               struct ao2_iterator user_iter;
                int concise = (a->argc == 4 && (!strcasecmp(a->argv[3], "concise")));
                /* List all the users in a conference */
                if (AST_LIST_EMPTY(&confs)) {
@@ -1311,7 +1344,8 @@ static char *meetme_show_cmd(struct ast_cli_entry *e, int cmd, struct ast_cli_ar
                }
                /* Show all the users */
                time(&now);
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
+               user_iter = ao2_iterator_init(cnf->usercontainer, 0);
+               while((user = ao2_iterator_next(&user_iter))) {
                        hr = (now - user->jointime) / 3600;
                        min = ((now - user->jointime) % 3600) / 60;
                        sec = (now - user->jointime) % 60;
@@ -1338,7 +1372,9 @@ static char *meetme_show_cmd(struct ast_cli_entry *e, int cmd, struct ast_cli_ar
                                        user->adminflags & ADMINFLAG_T_REQUEST ? "1" : "",
                                        user->talking, hr, min, sec);
                        }
+                       ao2_ref(user, -1);
                }
+               ao2_iterator_destroy(&user_iter);
                if (!concise) {
                        ast_cli(a->fd, "%d users in that conference.\n", cnf->users);
                }
@@ -1699,6 +1735,9 @@ static int conf_free(struct ast_conference *conf)
        if (conf->recordingformat) {
                ast_free(conf->recordingformat);
        }
+       if (conf->usercontainer) {
+               ao2_ref(conf->usercontainer, -1);
+       }
        ast_mutex_destroy(&conf->playlock);
        ast_mutex_destroy(&conf->listenlock);
        ast_mutex_destroy(&conf->recordthreadlock);
@@ -1712,13 +1751,19 @@ static void conf_queue_dtmf(const struct ast_conference *conf,
        const struct ast_conf_user *sender, struct ast_frame *f)
 {
        struct ast_conf_user *user;
+       struct ao2_iterator user_iter;
 
-       AST_LIST_TRAVERSE(&conf->userlist, user, list) {
-               if (user == sender)
+       user_iter = ao2_iterator_init(conf->usercontainer, 0);
+       while ((user = ao2_iterator_next(&user_iter))) {
+               if (user == sender) {
+                       ao2_ref(user, -1);
                        continue;
+               }
                if (ast_write(user->chan, f) < 0)
                        ast_log(LOG_WARNING, "Error writing frame to channel %s\n", user->chan->name);
+               ao2_ref(user, -1);
        }
+       ao2_iterator_destroy(&user_iter);
 }
 
 static void sla_queue_event_full(enum sla_event_type type, 
@@ -2052,8 +2097,9 @@ static int conf_run(struct ast_channel *chan, struct ast_conference *conf, int c
        int setusercount = 0;
        int confsilence = 0, totalsilence = 0;
 
-       if (!(user = ast_calloc(1, sizeof(*user))))
+       if (!(user = ao2_alloc(sizeof(*user), NULL))) {
                return ret;
+       }
 
        /* Possible timeout waiting for marked user */
        if ((confflags & CONFFLAG_WAITMARKED) &&
@@ -2216,22 +2262,21 @@ static int conf_run(struct ast_channel *chan, struct ast_conference *conf, int c
 
        ast_mutex_lock(&conf->playlock);
 
-       if (AST_LIST_EMPTY(&conf->userlist))
-               user->user_no = 1;
-       else
-               user->user_no = AST_LIST_LAST(&conf->userlist)->user_no + 1;
-
-       if (rt_schedule && conf->maxusers)
+       if (rt_schedule && conf->maxusers) {
                if (conf->users >= conf->maxusers) {
                        /* Sorry, but this confernce has reached the participant limit! */      
                        if (!ast_streamfile(chan, "conf-full", chan->language))
                                ast_waitstream(chan, "");
                        ast_mutex_unlock(&conf->playlock);
-                       user->user_no = 0;
                        goto outrun;
                }
+       }
 
-       AST_LIST_INSERT_TAIL(&conf->userlist, user, list);
+       ao2_lock(conf->usercontainer);
+       ao2_callback(conf->usercontainer, OBJ_NODATA, user_max_cmp, &user->user_no);
+       user->user_no++;
+       ao2_link(conf->usercontainer, user);
+       ao2_unlock(conf->usercontainer);
 
        user->chan = chan;
        user->userflags = confflags;
@@ -2996,17 +3041,21 @@ static int conf_run(struct ast_channel *chan, struct ast_conference *conf, int c
                                                                }
                                                                break;
                                                        case '3': /* Eject last user */
+                                                       {
+                                                               int max_no = 0;
                                                                menu_active = 0;
-                                                               usr = AST_LIST_LAST(&conf->userlist);
-                                                               if ((usr->chan->name == chan->name) || (usr->userflags & CONFFLAG_ADMIN)) {
-                                                                       if (!ast_streamfile(chan, "conf-errormenu", chan->language)) {
+                                                               ao2_callback(conf->usercontainer, OBJ_NODATA, user_max_cmp, &max_no);
+                                                               usr = ao2_find(conf->usercontainer, &max_no, 0);
+                                                               if ((usr->chan->name == chan->name)||(usr->userflags & CONFFLAG_ADMIN)) {
+                                                                       if(!ast_streamfile(chan, "conf-errormenu", chan->language))
                                                                                ast_waitstream(chan, "");
-                                                                       }
                                                                } else {
                                                                        usr->adminflags |= ADMINFLAG_KICKME;
                                                                }
+                                                               ao2_ref(user, -1);
                                                                ast_stopstream(chan);
-                                                               break;  
+                                                               break;
+                                                       }
                                                        case '4':
                                                                tweak_listen_volume(user, VOL_DOWN);
                                                                break;
@@ -3325,7 +3374,9 @@ bailoutandtrynormal:
                ast_dsp_free(dsp);
        }
        
-       if (user->user_no) { /* Only cleanup users who really joined! */
+       if (!user->user_no) {
+               ao2_ref(user, -1);
+       } else { /* Only cleanup users who really joined! */
                now = ast_tvnow();
                hr = (now.tv_sec - user->jointime) / 3600;
                min = ((now.tv_sec - user->jointime) % 3600) / 60;
@@ -3362,8 +3413,8 @@ bailoutandtrynormal:
                                conf->markedusers--;
                        }
                }
-               /* Remove ourselves from the list */
-               AST_LIST_REMOVE(&conf->userlist, user, list);
+               /* Remove ourselves from the container */
+               ao2_unlink(conf->usercontainer, user); 
 
                /* Change any states */
                if (!conf->users) {
@@ -3379,7 +3430,6 @@ bailoutandtrynormal:
                        pbx_builtin_setvar_helper(chan, "MEETMEBOOKID", conf->bookid);
                }
        }
-       ast_free(user);
        AST_LIST_UNLOCK(&confs);
 
        return ret;
@@ -3980,14 +4030,83 @@ static struct ast_conf_user *find_user(struct ast_conference *conf, char *caller
        
        sscanf(callerident, "%30i", &cid);
        if (conf && callerident) {
-               AST_LIST_TRAVERSE(&conf->userlist, user, list) {
-                       if (cid == user->user_no)
-                               return user;
-               }
+               user = ao2_find(conf->usercontainer, &cid, 0);
+               /* reference decremented later in admin_exec */
+               return user;
        }
        return NULL;
 }
 
+static int user_set_kickme_cb(void *obj, void *unused, int flags)
+{
+       struct ast_conf_user *user = obj;
+       user->adminflags |= ADMINFLAG_KICKME;
+       return 0;
+}
+
+static int user_set_muted_cb(void *obj, void *unused, int flags)
+{
+       struct ast_conf_user *user = obj;
+       if (!(user->userflags & CONFFLAG_ADMIN)) {
+               user->adminflags |= ADMINFLAG_MUTED;
+       }
+       return 0;
+}
+
+static int user_set_unmuted_cb(void *obj, void *unused, int flags)
+{
+       struct ast_conf_user *user = obj;
+       user->adminflags &= ~(ADMINFLAG_MUTED | ADMINFLAG_SELFMUTED);
+       return 0;
+}
+
+static int user_listen_volup_cb(void *obj, void *unused, int flags)
+{
+       struct ast_conf_user *user = obj;
+       tweak_listen_volume(user, VOL_UP);
+       return 0;
+}
+
+static int user_listen_voldown_cb(void *obj, void *unused, int flags)
+{
+       struct ast_conf_user *user = obj;
+       tweak_listen_volume(user, VOL_DOWN);
+       return 0;
+}
+
+static int user_talk_volup_cb(void *obj, void *unused, int flags)
+{
+       struct ast_conf_user *user = obj;
+       tweak_talk_volume(user, VOL_UP);
+       return 0;
+}
+
+static int user_talk_voldown_cb(void *obj, void *unused, int flags)
+{
+       struct ast_conf_user *user = obj;
+       tweak_talk_volume(user, VOL_DOWN);
+       return 0;
+}
+
+static int user_reset_vol_cb(void *obj, void *unused, int flags)
+{
+       struct ast_conf_user *user = obj;
+       reset_volumes(user);
+       return 0;
+}
+
+static int user_chan_cb(void *obj, void *args, int flags)
+{
+       struct ast_conf_user *user = obj;
+       const char *channel = args;
+
+       if (!strcmp(user->chan->name, channel)) {
+               return (CMP_MATCH | CMP_STOP);
+       }
+
+       return 0;
+}
+
 /*! \brief The MeetMeadmin application */
 /* MeetMeAdmin(confno, command, caller) */
 static int admin_exec(struct ast_channel *chan, void *data) {
@@ -4031,8 +4150,14 @@ static int admin_exec(struct ast_channel *chan, void *data) {
 
        ast_atomic_fetchadd_int(&cnf->refcount, 1);
 
-       if (args.user)
+       if (args.user) {
                user = find_user(cnf, args.user);
+               if (!user) {
+                       ast_log(LOG_NOTICE, "Specified User not found!\n");
+                       res = -2;
+                       goto usernotfound;
+               }
+       }
 
        switch (*args.command) {
        case 76: /* L: Lock */ 
@@ -4042,18 +4167,22 @@ static int admin_exec(struct ast_channel *chan, void *data) {
                cnf->locked = 0;
                break;
        case 75: /* K: kick all users */
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list)
-                       user->adminflags |= ADMINFLAG_KICKME;
+               ao2_callback(cnf->usercontainer, OBJ_NODATA, user_set_kickme_cb, NULL);
                break;
        case 101: /* e: Eject last user*/
-               user = AST_LIST_LAST(&cnf->userlist);
+       {
+               int max_no = 0;
+               ao2_callback(cnf->usercontainer, OBJ_NODATA, user_max_cmp, &max_no);
+               user = ao2_find(cnf->usercontainer, &max_no, 0);
                if (!(user->userflags & CONFFLAG_ADMIN))
                        user->adminflags |= ADMINFLAG_KICKME;
                else {
                        res = -1;
                        ast_log(LOG_NOTICE, "Not kicking last user, is an Admin!\n");
                }
+               ao2_ref(user, -1);
                break;
+       }
        case 77: /* M: Mute */ 
                if (user) {
                        user->adminflags |= ADMINFLAG_MUTED;
@@ -4063,97 +4192,46 @@ static int admin_exec(struct ast_channel *chan, void *data) {
                }
                break;
        case 78: /* N: Mute all (non-admin) users */
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
-                       if (!(user->userflags & CONFFLAG_ADMIN)) {
-                               user->adminflags |= ADMINFLAG_MUTED;
-                       }
-               }
+               ao2_callback(cnf->usercontainer, OBJ_NODATA, user_set_muted_cb, NULL);
                break;                                  
        case 109: /* m: Unmute */ 
-               if (user) {
-                       user->adminflags &= ~(ADMINFLAG_MUTED | ADMINFLAG_SELFMUTED | ADMINFLAG_T_REQUEST);
-               } else {
-                       res = -2;
-                       ast_log(LOG_NOTICE, "Specified User not found!\n");
-               }
+               user->adminflags &= ~(ADMINFLAG_MUTED | ADMINFLAG_SELFMUTED | ADMINFLAG_T_REQUEST);
                break;
        case 110: /* n: Unmute all users */
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
-                       user->adminflags &= ~(ADMINFLAG_MUTED | ADMINFLAG_SELFMUTED | ADMINFLAG_T_REQUEST);
-               }
+               ao2_callback(cnf->usercontainer, OBJ_NODATA, user_set_unmuted_cb, NULL);
                break;
        case 107: /* k: Kick user */ 
-               if (user) {
-                       user->adminflags |= ADMINFLAG_KICKME;
-               } else {
-                       res = -2;
-                       ast_log(LOG_NOTICE, "Specified User not found!\n");
-               }
+               user->adminflags |= ADMINFLAG_KICKME;
                break;
        case 118: /* v: Lower all users listen volume */
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
-                       tweak_listen_volume(user, VOL_DOWN);
-               }
+               ao2_callback(cnf->usercontainer, OBJ_NODATA, user_listen_voldown_cb, NULL);
                break;
        case 86: /* V: Raise all users listen volume */
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
-                       tweak_listen_volume(user, VOL_UP);
-               }
+               ao2_callback(cnf->usercontainer, OBJ_NODATA, user_listen_volup_cb, NULL);
                break;
        case 115: /* s: Lower all users speaking volume */
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
-                       tweak_talk_volume(user, VOL_DOWN);
-               }
+               ao2_callback(cnf->usercontainer, OBJ_NODATA, user_talk_voldown_cb, NULL);
                break;
        case 83: /* S: Raise all users speaking volume */
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
-                       tweak_talk_volume(user, VOL_UP);
-               }
+               ao2_callback(cnf->usercontainer, OBJ_NODATA, user_talk_volup_cb, NULL);
                break;
        case 82: /* R: Reset all volume levels */
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
-                       reset_volumes(user);
-               }
+               ao2_callback(cnf->usercontainer, OBJ_NODATA, user_reset_vol_cb, NULL);
                break;
        case 114: /* r: Reset user's volume level */
-               if (user) {
-                       reset_volumes(user);
-               } else {
-                       res = -2;
-                       ast_log(LOG_NOTICE, "Specified User not found!\n");
-               }
+               reset_volumes(user);
                break;
        case 85: /* U: Raise user's listen volume */
-               if (user) {
-                       tweak_listen_volume(user, VOL_UP);
-               } else {
-                       res = -2;
-                       ast_log(LOG_NOTICE, "Specified User not found!\n");
-               }
+               tweak_listen_volume(user, VOL_UP);
                break;
        case 117: /* u: Lower user's listen volume */
-               if (user) {
-                       tweak_listen_volume(user, VOL_DOWN);
-               } else {
-                       res = -2;
-                       ast_log(LOG_NOTICE, "Specified User not found!\n");
-               }
+               tweak_listen_volume(user, VOL_DOWN);
                break;
        case 84: /* T: Raise user's talk volume */
-               if (user) {
-                       tweak_talk_volume(user, VOL_UP);
-               } else {
-                       res = -2;
-                       ast_log(LOG_NOTICE, "Specified User not found!\n");
-               }
+               tweak_talk_volume(user, VOL_UP);
                break;
        case 116: /* t: Lower user's talk volume */
-               if (user) {
-                       tweak_talk_volume(user, VOL_DOWN);
-               } else {
-                       res = -2;
-                       ast_log(LOG_NOTICE, "Specified User not found!\n");
-               }
+               tweak_talk_volume(user, VOL_DOWN);
                break;
        case 'E': /* E: Extend conference */
                if (rt_extend_conf(args.confno)) {
@@ -4162,6 +4240,11 @@ static int admin_exec(struct ast_channel *chan, void *data) {
                break;
        }
 
+       if (args.user) {
+               /* decrement reference from find_user */
+               ao2_ref(user, -1);
+       }
+usernotfound:
        AST_LIST_UNLOCK(&confs);
 
        dispose_conf(cnf);
@@ -4201,9 +4284,8 @@ static int channel_admin_exec(struct ast_channel *chan, void *data) {
 
        AST_LIST_LOCK(&confs);
        AST_LIST_TRAVERSE(&confs, conf, list) {
-               AST_LIST_TRAVERSE(&conf->userlist, user, list) {
-                       if (!strcmp(user->chan->name, args.channel))
-                               break;
+               if ((user = ao2_callback(conf->usercontainer, 0, user_chan_cb, args.channel))) {
+                       break;
                }
        }
        
@@ -4228,7 +4310,7 @@ static int channel_admin_exec(struct ast_channel *chan, void *data) {
                        ast_log(LOG_WARNING, "Unknown MeetMeChannelAdmin command '%s'\n", args.command);
                        break;
        }
-
+       ao2_ref(user, -1);
        AST_LIST_UNLOCK(&confs);
        
        return 0;
@@ -4272,9 +4354,7 @@ static int meetmemute(struct mansession *s, const struct message *m, int mute)
                return 0;
        }
 
-       AST_LIST_TRAVERSE(&conf->userlist, user, list)
-               if (user->user_no == userno)
-                       break;
+       user = ao2_find(conf->usercontainer, &userno, 0);
 
        if (!user) {
                AST_LIST_UNLOCK(&confs);
@@ -4291,6 +4371,7 @@ static int meetmemute(struct mansession *s, const struct message *m, int mute)
 
        ast_log(LOG_NOTICE, "Requested to %smute conf %s user %d userchan %s uniqueid %s\n", mute ? "" : "un", conf->confno, user->user_no, user->chan->name, user->chan->uniqueid);
 
+       ao2_ref(user, -1);
        astman_send_ack(s, m, mute ? "User muted" : "User unmuted");
        return 0;
 }
@@ -4320,6 +4401,7 @@ static int action_meetmelist(struct mansession *s, const struct message *m)
        char idText[80] = "";
        struct ast_conference *cnf;
        struct ast_conf_user *user;
+       struct ao2_iterator user_iter;
        int total = 0;
 
        if (!ast_strlen_zero(actionid))
@@ -4335,12 +4417,13 @@ static int action_meetmelist(struct mansession *s, const struct message *m)
        /* Find the right conference */
        AST_LIST_LOCK(&confs);
        AST_LIST_TRAVERSE(&confs, cnf, list) {
+               user_iter = ao2_iterator_init(cnf->usercontainer, 0);
                /* If we ask for one particular, and this isn't it, skip it */
                if (!ast_strlen_zero(conference) && strcmp(cnf->confno, conference))
                        continue;
 
                /* Show all the users */
-               AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
+               while ((user = ao2_iterator_next(&user_iter))) {
                        total++;
                        astman_append(s,
                        "Event: MeetmeList\r\n"
@@ -4366,8 +4449,10 @@ static int action_meetmelist(struct mansession *s, const struct message *m)
                        user->userflags & CONFFLAG_MONITOR ? "Listen only" : user->userflags & CONFFLAG_TALKER ? "Talk only" : "Talk and listen",
                        user->userflags & CONFFLAG_MARKEDUSER ? "Yes" : "No",
                        user->adminflags & ADMINFLAG_MUTED ? "By admin" : user->adminflags & ADMINFLAG_SELFMUTED ? "By self" : "No",
-                       user->talking > 0 ? "Yes" : user->talking == 0 ? "No" : "Not monitored"); 
+                       user->talking > 0 ? "Yes" : user->talking == 0 ? "No" : "Not monitored");
+                       ao2_ref(user, -1); 
                }
+               ao2_iterator_destroy(&user_iter);
        }
        AST_LIST_UNLOCK(&confs);
        /* Send final confirmation */