[Asterisk-Dev] Race issue in channel.c involving uniqueint on Asterisk 1.2.1

[Werner Johansson]

Hello everyone,

We have faced a problem with non-unique UniqueID:s attached to channels when 
originating lots of calls at once in async mode using the manager interface. 
This creates problems for us trying to map channel events like link and call 
progress to a specific ActionID being supplied to the Originate action. 
Tracking down the code that calculates the uniqueid (in channel.c) and 
looking at the code generated by the compiler it is obvious that this piece 
of code isn't thread safe (non-atomic increment of the call id). If this 
happens before the time variable changes there will be more than one channel 
with the same ID, which obviously is bad. If the UniqueID is never used this 
probably doesn't affect operation, but for us that is the only way to see 
which channel belongs to which call, as the ActionID isn't passed down to 
the actual channel structure (i.e we only get ActionID on OriginateSuccess, 
but never on Newstate for instance).

This has been spotted in Asterisk 1.2.1, but no changes to this code can be 
seen in the newer revisions on channel.c in the trunk or on the 1.2 branch. 
We have developed a quick fix which seems to work, but there might be some 
other way to implement this. Diff included below, comments appreciated!

Should this be reported as a bug or has it already been seen elsewhere? I 
haven't seen uniqueint being mentioned as a source of problems before.

Best Regards,
Werner Johansson



*** channel.c   2005-12-02 00:34:58.000000000 +0100
--- ../../ast1.2_rh/asterisk-1.2.1/channel.c    2005-12-29 
22:48:24.717558008 +0100
***************
*** 97,100 ****
--- 97,108 ----
  static int uniqueint = 0;

+ /* Rådhuset edit starts - CJS 051229 21:45 */
+
+ AST_MUTEX_DEFINE_STATIC(ast_unique_increase_lock);
+
+ /* Rådhuset edit ends - CJS 051229 21:45 */
+
+
+
  unsigned long global_fin = 0, global_fout = 0;

***************
*** 584,588 ****
        tmp->fin = global_fin;
        tmp->fout = global_fout;
!       snprintf(tmp->uniqueid, sizeof(tmp->uniqueid), "%li.%d", (long) 
time(NULL), uniqueint++);
        headp = &tmp->varshead;
        ast_mutex_init(&tmp->lock);
--- 592,606 ----
        tmp->fin = global_fin;
        tmp->fout = global_fout;
!
!       /* Rådhuset edit starts - CJS 051229 21:45 */
!
!       ast_mutex_lock(&ast_unique_increase_lock);
!       uniqueint++;
!       snprintf(tmp->uniqueid, sizeof(tmp->uniqueid), "%li.%d", (long) 
time(NULL),uniqueint);
!       ast_mutex_unlock(&ast_unique_increase_lock);
!
!       /* Rådhuset edit ends - CJS 051229 21:45 */
!
!
        headp = &tmp->varshead;
        ast_mutex_init(&tmp->lock);