Summary:	ASTERISK-12052: [patch] Possible deadlock: rwlock can't be recursively locked (writelocked)
Reporter:	Yuri (ys)	Labels:
Date Opened:	2008-05-20 08:52:03	Date Closed:	2008-06-16 16:12:41
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	PBX/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) pbx.c.diff ( 1) test.c
Description:	I found, that some thread libs, block thread if one rwlock are locked twice. But some libs have "Resource deadlock avoided" decision. This situation present if called __ast_context_destroy() function (main/pbc.c). In this function, context is locked and called ast_context_remove_extension2() function, there context is locked again. ****** ADDITIONAL INFORMATION ****** Qoute from Linux thread man page: "The calling thread may deadlock if at the time the call is made it holds the read-write lock (whether a read or write lock)." Quote from FreeBSD thread man page: "The results are undefined if the calling thread already holds the lock at the time the call is made." I make test case: test.c and got the following result: (FreeBSD) Reentrant C Library (libc_r,  gcc -o test test.c -lc_r): pthread_rwlock_trywrlock: Device busy [deadlock] (ctrl-C pressed) (FreeBSD) POSIX Threads Library (libpthread, gcc -o test test.c -lpthread): pthread_rwlock_trywrlock: Device busy [deadlock] (ctrl-C pressed) (FreeBSD) 1:1 Threading Library (libthr, gcc -o test test.c -lthr): pthread_rwlock_trywrlock: Device busy pthread_rwlock_wrlock 2: Resource deadlock avoided (Linux) POSIX.1 ( gcc -otest test.c -pthread): pthread_rwlock_trywrlock: Device busy pthread_rwlock_wrlock 2: Resource deadlock avoided
Comments:	By: Yuri (ys) 2008-05-22 05:40:53 I upload small patch, what temporary fix double context rwlock in pbx.c By: Digium Subversion (svnbot) 2008-06-16 15:36:58 Repository: asterisk Revision: 123165 U   trunk/apps/app_dial.c U   trunk/apps/app_queue.c U   trunk/apps/app_stack.c U   trunk/include/asterisk/pbx.h U   trunk/main/features.c U   trunk/main/pbx.c ------------------------------------------------------------------------ r123165 | murf | 2008-06-16 15:36:56 -0500 (Mon, 16 Jun 2008) | 19 lines (closes issue ASTERISK-12052) Reported by: ys Many thanks to ys for doing the research on this problem. I didn't think it would be best to unlock the contexts and then relock them after the remove_extension2() call, so I added an extra arg to remove_extension2() and set it appropriately in each call. There were not that many. I considered forcing the code to lock the contexts before the call to remove_extension2(), but that would require a slightly greater degree of changes, especially since the find_context_locked is local to pbx.c I did a simple sanity test to make sure the code doesn't mess things up in general. ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=123165 By: Digium Subversion (svnbot) 2008-06-16 16:12:41 Repository: asterisk Revision: 123173 _U  branches/1.6.0/ U   branches/1.6.0/apps/app_dial.c U   branches/1.6.0/apps/app_queue.c U   branches/1.6.0/apps/app_stack.c U   branches/1.6.0/include/asterisk/pbx.h U   branches/1.6.0/main/features.c U   branches/1.6.0/main/pbx.c ------------------------------------------------------------------------ r123173 | murf | 2008-06-16 16:12:40 -0500 (Mon, 16 Jun 2008) | 27 lines Merged revisions 123165 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r123165 | murf | 2008-06-16 14:43:46 -0600 (Mon, 16 Jun 2008) | 19 lines (closes issue ASTERISK-12052) Reported by: ys Many thanks to ys for doing the research on this problem. I didn't think it would be best to unlock the contexts and then relock them after the remove_extension2() call, so I added an extra arg to remove_extension2() and set it appropriately in each call. There were not that many. I considered forcing the code to lock the contexts before the call to remove_extension2(), but that would require a slightly greater degree of changes, especially since the find_context_locked is local to pbx.c I did a simple sanity test to make sure the code doesn't mess things up in general. ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=123173