Summary:ASTERISK-08595: Race condition in app_meetme when using the 'e' (empty) and 'd' (dynamic) option and two calls arrive at the "same" time.
Reporter:Eliel Sardanons (eliel)Labels:
Date Opened:2007-01-17 09:59:09.000-0600Date Closed:2007-07-09 21:20:43
Versions:Frequency of
Environment:Attachments:( 0) app_meetme.c.patch
Description:When we have an extensions like this:

exten => s,1,MeetMe(|de)

and two calls arrive at the "same time", the first call enters to meetme number '0' and the second call "enters to the same conference! (number 0)" and the second  call should enter to conference '1'.
Doing some code review i notice that the "map[1024]" variable is populated in conf_exec() but then when it needs to be checked in the body of this function could be obsolete (in fact is obsolete for the second call).


I couldn?t make a patch to solve this issue in a simple way, what I have done was to reduce the probability of a race condition, making "map[1024]" a global variable,  and then set map[confno_int] = 1 when a conference number is allocated and map[confno_int] = 0 when conf_free is called.
Comments:By: Eliel Sardanons (eliel) 2007-02-06 09:05:23.000-0600

The meetme application is run through an AGI but this could be avoid.

------------------------- CLI OUTPUT ---------------------------
   AGI Script Executing Application: (MEETME) Options: (|edpqx)
   -- Playing 'conf-enteringno' (language 'es')
   -- AGI Script Executing Application: (MEETME) Options: (|edpqx)
   -- Playing 'conf-enteringno' (language 'es')
   -- AGI Script Executing Application: (MEETME) Options: (|edpqx)
   -- Playing 'conf-enteringno' (language 'es')
   -- Playing 'digits/0' (language 'es')
   -- Playing 'digits/0' (language 'es')
   -- Playing 'digits/0' (language 'es')
   -- Created MeetMe conference 1023 for conference '0'
   -- Started music on hold, class 'default', on SIP/1141091702-098bbd78
   -- Stopped music on hold on SIP/1141091702-098bbd78

----------------------- EOF CLI --------------------------------------

As you could see when 2 (or in this example 3) calls arrive at "the same time", they are assigned the same conference number!.

By: Serge Vecher (serge-v) 2007-03-07 11:23:44.000-0600

what happens with 1.4.1?

By: Eliel Sardanons (eliel) 2007-03-08 11:06:08.000-0600

I will try it and give you feedback...

By: Eliel Sardanons (eliel) 2007-03-20 10:00:38

------------------- ASTERISK 1.4.1 --------------------
   -- Executing [3333@from-internal:1] MeetMe("SIP/3005-0a00db80", "|ed") in new stack
   -- <SIP/3005-0a00db80> Playing 'conf-enteringno' (language 'en')
   -- Executing [3333@from-internal:1] MeetMe("SIP/3013-09ff2640", "|ed") in new stack
   -- <SIP/3013-09ff2640> Playing 'conf-enteringno' (language 'en')
   -- Executing [3333@from-internal:1] MeetMe("SIP/3008-0a043ec0", "|ed") in new stack
   -- <SIP/3008-0a043ec0> Playing 'conf-enteringno' (language 'en')
   -- <SIP/3005-0a00db80> Playing 'digits/0' (language 'en')
   -- <SIP/3013-09ff2640> Playing 'digits/0' (language 'en')
   -- <SIP/3008-0a043ec0> Playing 'digits/0' (language 'en')
   -- Created MeetMe conference 1023 for conference '0'

By: Eliel Sardanons (eliel) 2007-03-20 16:47:06

The problem I think is simple to notice.
We populate array 'map[]' at the "beginning" of conf_exec(), and then we use it (or not it depends on the params) at the middle of conf_exec(), so, 'map[]' has old data.
What we have to do, to solve this issue (I think, and I did something like this to prevent this race condition) is make 'map[]' a global variable, then we need to lock the access to 'map[]' and when we go over 'map[]' too (like a for()). I'm not sure if this will solve the problem at all, but we will reduce the probability of the race condition to occur. I made this change and I couldn?t reproduce it any more.

By: Eliel Sardanons (eliel) 2007-03-23 14:53:30

This is how I solved the issue, but I don?t know if it is the best way or bug free.

By: Eliel Sardanons (eliel) 2007-03-23 15:25:06

Disclaimer on File? YES

By: Joshua C. Colp (jcolp) 2007-03-29 12:43:19

Fixed in 1.2 as of revision 59360, 1.4 as of revision 59361, and trunk as of revision 59362. Thanks!