Summary:	ASTERISK-07175: [patch] [post-1.4] "MacroExclusive" application - thread-safe Macro()
Reporter:	Steve Davies . (stevedavies)	Labels:
Date Opened:	2006-06-15 09:50:45	Date Closed:	2006-08-13 22:26:35
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Applications/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) macroexclusive.patch ( 1) macroexclusive.txt
Description:	Trying to access shared data in your Asterisk dialplan is problematic because of problems with races between multiple calls. MacroExclusive calls a macro just like "Macro" does, but uses a mutex to lock the macro so that other calls have to wait until we are done before they can enter the macro. You need this if you have Macros that perform "multi-step" operations on shared data like global variables, astdb or even an external database. As discussed on asterisk-dev; exposing mutexes indirectly like this rather than through "Mutex" app or whatever simplifies for the programmer - especially in respect of making sure Mutexes aren't left locked.  Any exit from the macro will ALWAYS unlock the mutex. I've attached some docs that are also added into the doc subdirectory by the patch. Disclaimer is on file! Regards, Steve ****** ADDITIONAL INFORMATION ****** have the patch for branch-1.2 if anyone wants it.
Comments:	By: Steve Davies . (stevedavies) 2006-06-15 10:16:11 thanks - better description! By: Steve Davies . (stevedavies) 2006-06-15 10:21:52 sorry - also should have mentioned that the patch adds a new file docs/macroexclusive.txt. By: Russell Bryant (russell) 2006-06-18 23:39:14 I was just thinking about something in regards to this.  If something already is holding the lock and a channel has to wait any amount of time to get the lock, autoservice should be turned on before trying to get the lock. So, before you call ast_context_lockmacro, you should call ast_autoservice_start(chan) and also ast_autoservice_stop(chan) after the lock is obtained.  This will ensure that incoming frames are read from the channel, even when blocking while waiting on the macro lock. And while you're making some changes, there are some small formatting and coding guidelines type issues: - In your new functions for ast_context_lock/unlockmacro, there is a way to simplify the loop. struct ast_context *c = NULL; ... while ((c = ast_walk_contexts(c))) { if (!strcmp(ast_get_context_name(c), context)) { ret = 0; break; } } - There are a few places where you have a statement on the same line as an if statement.  The statement after the conditional should be placed on its own line. - in the lock/unlockmacro functions, they are checking the result of ast_mutex_lock/unlock.  We have standardized on ignoring the return value from those functions.  These operations will always be successful since we're using recursive mutexes. - Here is an update to your doxygen documentation to make better use of doxygen keywords: /*! \brief locks the macrolock in the given given context * \param macrocontext name of the macro-context to lock * * Locks the given macro-context to ensure only one thread (call) can execute it at a time * * \retval 0 on success * \retval -1 on failure */ int ast_context_lockmacro(const char *macrocontext); /*! * \brief Unlocks the macrolock in the given context * \param macrocontext name of the macro-context to unlock * * Unlocks the given macro-context so that another thread (call) can execute it * * \retval 0 on success * \retval -1 on failure */ int ast_context_unlockmacro(const char *macrocontext); By: Steve Davies . (stevedavies) 2006-07-05 04:59:11 Hi Russell, Thanks for the comments. MacroExclusive really was not conceived for long-running macros - but per the docs just for protecting shared data structures.  Still, no doubt it will be misused so I'll do your autoservice suggestions. As for the while loop, I guess my old Pascal habits are showing... I'll put up a new version of the patch soon as I get a chance. Steve By: Russell Bryant (russell) 2006-08-13 22:21:36 Also, the MacroExclusive application is not unregistered on module unload. :) By: Russell Bryant (russell) 2006-08-13 22:26:34 added to the trunk in revisions 39681 and 39683, with the modifications I discussed here, thanks!