[asterisk-bugs] [Asterisk 0012840]: [patch] Makes pbx_builtin_getvar_helper values thread safe

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=12840 
====================================================================== 
Reported By:                pputman
Assigned To:                
====================================================================== 
Project:                    Asterisk
Issue ID:                   12840
Category:                   Applications/app_dial
Reproducibility:            always
Severity:                   tweak
Priority:                   normal
Status:                     new
Asterisk Version:           SVN 
SVN Branch (only for SVN checkouts, not tarball releases):  trunk 
SVN Revision (number only!): 122043 
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             06-12-2008 05:39 CDT
Last Modified:              06-12-2008 16:35 CDT
====================================================================== 
Summary:                    [patch] Makes pbx_builtin_getvar_helper values
thread safe
Description: 
Uses ast_channel_lock and unlock, to make app_dial.c more threadsafe.
====================================================================== 

---------------------------------------------------------------------- 
 Corydon76 - 06-12-08 16:35  
---------------------------------------------------------------------- 
Still a couple of problems.  For example, in the second hunk, you'll see
that there's a "return NULL" within a conditional, where you aren't
unlocking the channel.  You will need to unlock the channel there, before
returning, in addition to unlocking if the conditional doesn't execute.  So
you need another line in the patch right there.

In the 5th hunk, it's assigning values from the channel to a configuration
structure.  Those elements need to be ast_strdupa()ed before unlocking the
channel, because it's not actually done with the variable; instead it's
just copying a pointer to the same space (which could be freed by another
thread immediately after you release the channel lock).

In the 6th hunk, the new code should actually cause a segfault, because
you're destroying the space that a pointer points to immediately after
retrieving it; you'd need to ast_strdupa() the pointer retrieved from
pbx_builtin_getvar_helper() BEFORE you set the same named variable to NULL
with pbx_builtin_setvar_helper() (the second operation causes the memory
pointed to by the first operation to be freed).

I haven't gone any further than this, but the patch still needs some work. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
06-12-08 16:35  Corydon76      Note Added: 0088649                          
======================================================================