| Summary: | ASTERISK-12179: [patch] Makes pbx_builtin_getvar_helper values thread safe | ||
| Reporter: | Patrick Putman (pputman) | Labels: | |
| Date Opened: | 2008-06-12 05:39:38 | Date Closed: | 2008-06-18 08:02:44 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Applications/app_dial |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) app_dial_threadsafe3.patch | |
| Description: | Uses ast_channel_lock and unlock, to make app_dial.c more threadsafe. | ||
| Comments: | By: Tilghman Lesher (tilghman) 2008-06-12 16:35: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. By: Patrick Putman (pputman) 2008-06-18 01:37:07 Made the changes recommended, I think it's a little better now. Thanks. By: Patrick Putman (pputman) 2008-06-18 02:55:43 #2 still had a problem I noticed, app_dial_threadsafe3.patch is the correct one. By: Digium Subversion (svnbot) 2008-06-18 08:02:05 Repository: asterisk Revision: 123648 U trunk/apps/app_dial.c ------------------------------------------------------------------------ r123648 | tilghman | 2008-06-18 08:02:04 -0500 (Wed, 18 Jun 2008) | 6 lines Channel lock janitor -- add locks around retrieval of channel variables (closes issue ASTERISK-12179) Reported by: pputman Patches: app_dial_threadsafe3.patch uploaded by pputman (license 81) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=123648 By: Digium Subversion (svnbot) 2008-06-18 08:02:44 Repository: asterisk Revision: 123649 _U branches/1.6.0/ ------------------------------------------------------------------------ r123649 | tilghman | 2008-06-18 08:02:44 -0500 (Wed, 18 Jun 2008) | 13 lines Blocked revisions 123648 via svnmerge ........ r123648 | tilghman | 2008-06-18 08:09:02 -0500 (Wed, 18 Jun 2008) | 6 lines Channel lock janitor -- add locks around retrieval of channel variables (closes issue ASTERISK-12179) Reported by: pputman Patches: app_dial_threadsafe3.patch uploaded by pputman (license 81) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=123649 | ||