<HTML>
<HEAD>
<TITLE>Re: [Freeswitch-dev] Switch_core_get_variable</TITLE>
</HEAD>
<BODY>
<FONT FACE="Verdana, Helvetica, Arial"><SPAN STYLE='font-size:12.0px'><BR>
I can enter it into jira.<BR>
<BR>
I’m not sure if I agree with your assessment. There is already a mutex lock/unlock around switch_core_get_variable’s internals today, no additional risk of deadlock. As for switch_channel_get_variable, which also has a mutex of it’s own, is already using switch_core_session_strdup before returning. I’m proposing the equivalent to switch_channel_get_variable_dup be added for switch_core_get_variable. To me it seems we must have this if you want to reliably read a variable that might change at runtime.<BR>
<BR>
Kevin<BR>
<BR>
<BR>
<BR>
<BR>
On 2/2/11 12:01 PM, "Steven Ayre" <steveayre@gmail.com> wrote:<BR>
<BR>
</SPAN></FONT><BLOCKQUOTE><BLOCKQUOTE><FONT FACE="Verdana, Helvetica, Arial"><SPAN STYLE='font-size:12.0px'>I found a hole in the way we handle core variables in FS. Switch_core_get_variable does the lookup and returns the found pointer. In Switch_core_set_variable the first step it does is look up the variable and free it (if it exists). This would free it out from under another that has just done a get on it. This is how I stumbled on this. <BR>
</SPAN></FONT></BLOCKQUOTE><FONT FACE="Verdana, Helvetica, Arial"><SPAN STYLE='font-size:12.0px'><BR>
The correct place to report bugs is the bug tracker: <a href="http://jira.freeswitch.org/">http://jira.freeswitch.org/</a><BR>
<BR>
It's a known problem. Basically the answer is you shouldn't really be changing core variables after FS has loaded, at least not while handling traffic. If you're doing so as part of your application, you're doing it wrong. mod_db is probably better suited to what you want to do.<BR>
<BR>
</SPAN></FONT><BLOCKQUOTE><FONT FACE="Verdana, Helvetica, Arial"><SPAN STYLE='font-size:12.0px'>Is the right fix is to add a switch_core_get_variable_dup that dups the string while in the mutex protection? I realize in the core case this will require the caller to then free the returned memory, but this is better than getting a bad pointer. This is analogous to the switch_channel_get_variable and it’s _dup implementation, although it dups it to session memory.<BR>
</SPAN></FONT></BLOCKQUOTE><FONT FACE="Verdana, Helvetica, Arial"><SPAN STYLE='font-size:12.0px'><BR>
Just my 2 cents (I'm not a core developer)... That could be done. But it's also slower because it means dynamically assigning memory and then copying the string. And it can't be cached. And the mutex will make all calls block, not just the single call as with the channel variable - which might possibly introduce a risk of deadlock.<BR>
<BR>
-Steve<BR>
<BR>
<BR>
<BR>
On 2 February 2011 19:39, Kevin Snow <kevin.snow@ooma.com> wrote:<BR>
</SPAN></FONT><BLOCKQUOTE><FONT FACE="Verdana, Helvetica, Arial"><SPAN STYLE='font-size:12.0px'><BR>
I found a hole in the way we handle core variables in FS. Switch_core_get_variable does the lookup and returns the found pointer. In Switch_core_set_variable the first step it does is look up the variable and free it (if it exists). This would free it out from under another that has just done a get on it. This is how I stumbled on this. <BR>
<BR>
Is the right fix is to add a switch_core_get_variable_dup that dups the string while in the mutex protection? I realize in the core case this will require the caller to then free the returned memory, but this is better than getting a bad pointer. This is analogous to the switch_channel_get_variable and it’s _dup implementation, although it dups it to session memory.<BR>
<BR>
Switch_channel_get_variable’s ability to peak through to the core variables is susceptible to this. If a core variable is changed after switch_channel_get_variable looks it up but before it dups to the session pool, it’ll have a bad pointer. <BR>
<FONT COLOR="#888888"><BR>
Kevin Snow<BR>
Ooma, Inc</FONT> <BR>
<BR>
_______________________________________________<BR>
FreeSWITCH-dev mailing list<BR>
FreeSWITCH-dev@lists.freeswitch.org<BR>
<a href="http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev">http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev</a><BR>
UNSUBSCRIBE:<a href="http://lists.freeswitch.org/mailman/options/freeswitch-dev">http://lists.freeswitch.org/mailman/options/freeswitch-dev</a><BR>
<a href="http://www.freeswitch.org">http://www.freeswitch.org</a><BR>
<BR>
</SPAN></FONT></BLOCKQUOTE><FONT FACE="Verdana, Helvetica, Arial"><SPAN STYLE='font-size:12.0px'><BR>
<BR>
<HR ALIGN=CENTER SIZE="3" WIDTH="95%"></SPAN></FONT><SPAN STYLE='font-size:12.0px'><FONT FACE="Courier New">_______________________________________________<BR>
FreeSWITCH-dev mailing list<BR>
FreeSWITCH-dev@lists.freeswitch.org<BR>
<a href="http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev">http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev</a><BR>
UNSUBSCRIBE:<a href="http://lists.freeswitch.org/mailman/options/freeswitch-dev">http://lists.freeswitch.org/mailman/options/freeswitch-dev</a><BR>
<a href="http://www.freeswitch.org">http://www.freeswitch.org</a><BR>
</FONT></SPAN></BLOCKQUOTE><SPAN STYLE='font-size:12.0px'><FONT FACE="Courier New"><BR>
</FONT></SPAN>
</BODY>
</HTML>