Summary:	ASTERISK-05988: [RFC] LOCAL_USER_ADD / LOCAL_USER_REMOVE semantics ?
Reporter:	Luigi Rizzo (rizzo)	Labels:
Date Opened:	2006-01-05 12:07:39.000-0600	Date Closed:	2011-06-07 14:02:48
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	posted on the mailing list but got no answer. Just to be sure it does not get lost, or it is properly closed, here it is again a Request For Comments: I am a bit unclear on the exact semantics of LOCAL_USER_ADD and LOCAL_USER_REMOVE -- as i understand from the documentation, they are meant to keep track of the channels using a given module, both to signal them upon certain events, and to know when we can unload a module because the usecount is 0. However, for the latter we have a bit of a race, because both macros are called within the module itself, so localusecount can become 0 while a thread is still executing the final part of the handler. I suppose a better option would be to put the LOCAL_USER_ADD and LOCAL_USER_REMOVE calls around the call to the module's handler in pbx_exec(), ast_func_read() and ast_func_write() (if i haven't missed other places). Of course this would require to put the localuser stuff into the module/function descriptor, but in addition to giving correct behaviour, this change would also give a huge simplification in the module's code because there would be no need to replicate the LOCALUSER calls all over the place. Finally, the change can be made incrementally, by first adding the localuser things (maybe with a different name) in the wrapper, then redefining the LOCALUSER macros to empty, and then removing the now useless LOCALUSERS macros from the code as we sweep through it. Makes sense ?
Comments:	By: crich (crich) 2006-01-05 14:10:16.000-0600 Makes a lot of sense. By: Russell Bryant (russell) 2006-01-08 23:04:54.000-0600 In addition to applications and functions, we will have to update the use count for registered CLI commands and manager actions as well.  When these registered functions are in use, there is not an associated 'localuser' channel, but we still need to increase the use count. By: Tilghman Lesher (tilghman) 2006-01-08 23:50:04.000-0600 We'd need to mark each function that is invoked with register/unregister, really.  That's the only way to get everything.  This should probably include channels, and res_* modules.  Probably the best place to implement this would be in the redesign for the loader that you're doing elsewhere, since we'd need to know which functions needed a localuser_hangup when you attempt an unload on an arbitrary module. Then there's the invocation that you've already mentioned in the description. By: Olle Johansson (oej) 2006-01-09 01:18:46.000-0600 ...and we need doxygen docs for this :-) By: Luigi Rizzo (rizzo) 2006-01-09 01:32:14.000-0600 i need one clarification. I grep'ped for LOCAL_USER_ADD/LOCAL_USER_REMOVE in the source tree, and the only places where i see them are in functions called through pbx_exec(), ast_func_read() and ast_func_write(). So my initial suggestion was to put the call in these 3 wrapper functions and not in the individual methods. This is a completely equivalent replacement from a functional point of view (with the additional bonus of not decrementing the usecount when you are still running the code of the module). If i understand well, you are commenting that these calls alone (i.e. what is currently in the tree) are not enough to track all "things" (don't know how to call them - channels and cli commands and functions etc.) that need to be acted upon when there is a request to unload a module ? So what this means that upon such a request we are unable to signal all "things" to stop, hence the unload cannot proceed because the usecount will not go to 0 (at least not upon our request; maybe later, when the "thing" is done with its task). Then there is the other issue of usecounts on code which at the moment is not done correctly, but as you mention it is done in the loader redesign in the other report on mantis. By: Russell Bryant (russell) 2006-01-09 08:28:53.000-0600 Yes, that is what we are getting at.  The use count is not correctly tracked for all 'things' at this point.  If that is addressed in the loader re-design, great!  We can just focus on the localuser stuff here. As far as keeping track of the 'localuser' channels, I think you have it all covered. By: Olle Johansson (oej) 2006-01-30 14:07:31.000-0600 What is this bug report waiting for? Decision, update or what? /Housekeeping By: Luigi Rizzo (rizzo) 2006-01-30 14:45:42.000-0600 a word from the software architects would be certainly welcome. This said, the prerequisites are the commit of the localuser changes (ASTERISK-6059, two of the three chunks already in) and, to some degree, of the loader cleanup (ASTERISK-4275). By: Tilghman Lesher (tilghman) 2006-02-23 16:51:47.000-0600 Given that I think we have a game plan, this RFC should no longer be needed.  I'm closing it.