Summary:	ASTERISK-12327: [patch] dynamic thread identifiers may be duplicated
Reporter:	Peter Grayson (jpgrayson)	Labels:
Date Opened:	2008-07-07 10:58:02	Date Closed:	2008-07-07 11:55:06
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_iax2
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) chan_iax2_dyn_threadnum.patch
Description:	Currently in chan_iax2, the way that identifiers for dynamic threads "threadnum" are assigned leads to cases where multiple co-existing threads may have the same identifier. When a dynamic thread is created, its threadnum identifier is set to the global iaxdynamicthreadcount state. This iaxdynamicthreadcount is simply the count of currently existing dynamic threads. Using iaxdynamicthreadcount to assign threadnum provides no guarantee that each thread will have a unique thread number. It is actually possible to have the case where there are, for example, two dynamic threads with threadnum => 6, four threads with dynamic threadnum => 3, etc. This problem does not yield any functional deficiencies, but it does adversely impact debugging threads. It reduces the amount of information provided by "iax2 show threads".
Comments:	By: Peter Grayson (jpgrayson) 2008-07-07 11:05:40 Added a simple patch to resolve this issue. Use a new variable, iaxdynamicthreadnum, to assign dynamic threads' threadnum. This new variable monotonically increases thereby ensuring that each dynamic thread's threadnum is unique. By: Mark Michelson (mmichelson) 2008-07-07 11:15:57 As a precaution, it would probably be better to use ast_atomic_fetchadd_int instead of the ++ operator to increment the global variable. Thanks for the patch. I think the idea of a monotonically increasing identifier is a good approach. By: Peter Grayson (jpgrayson) 2008-07-07 11:41:39 The new iaxdynamicthreadnum variable is implicitly protected by having the dynamic_list lock. There is no actual chance of two threads manipulating iaxdynamicthreadnum at the same time. Also, iaxdynamicthreadcount similarly relies on the implicit serialization from the dynamic_list lock. I would just assume leave both of these variables alone. Otherwise I guess we'd really want to use ast_atomic_fetchadd() for both. By: Mark Michelson (mmichelson) 2008-07-07 11:50:19 Ah, you're quite right about the dynamic_list lock protecting the increment of the value. Thanks again for the patch. I'll get it committed as soon as possible. By: Digium Subversion (svnbot) 2008-07-07 11:55:04 Repository: asterisk Revision: 128639 U   branches/1.4/channels/chan_iax2.c ------------------------------------------------------------------------ r128639 | mmichelson | 2008-07-07 11:55:02 -0500 (Mon, 07 Jul 2008) | 10 lines By using the iaxdynamicthreadcount to identify a thread, it was possible for thread identifiers to be duplicated. By using a globally-unique monotonically- increasing integer, this is now avoided. (closes issue ASTERISK-12327) Reported by: jpgrayson Patches:      chan_iax2_dyn_threadnum.patch uploaded by jpgrayson (license 492) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=128639