Summary:	ASTERISK-14038: [patch] Change readq locking
Reporter:	Damien Wedhorn (wedhorn)	Labels:
Date Opened:	2009-04-29 15:32:26	Date Closed:	2011-07-26 14:17:14
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/Channels
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) readq.diff
Description:	Revision of chan->readq locking to utilise a list lock rather than the chan lock. The main advantage of this patch is that channels need not lock before calling ast_queue_frame or related functions purely to avoid a deadlock. Obvious changes to chan_sip and chan_iax have been included. I'm sure there are others. ****** ADDITIONAL INFORMATION ****** In order to not change behaviour, channels are still locked before calling ast_queue_hangup as if the channel is not locked, we can't reliably set _softhangup.
Comments:	By: David Brillert (aragon) 2010-02-16 15:29:03.000-0600 Is anyone going to post this to the reviewboard etc? By: David Brillert (aragon) 2010-02-16 16:01:08.000-0600 I'm guessing this patch once submitted to 1.4 would close two of my outstanding bug reports (both service affecting): ASTERISK-14994 ASTERISK-15611 By: David Brillert (aragon) 2010-02-17 11:04:09.000-0600 pinging reporter: Hi wedhorn, are you still working on this patch? Have you put this into production anywhere? By: Damien Wedhorn (wedhorn) 2010-02-17 13:22:47.000-0600 I haven't been doing much with asterisk for a while. Testing was limited and this patch certainly needs significant testing as it could easily lead to regressions in basically all channels. I'll try to bring the patch up to date and move it to the review board in the next couple of weeks. By: David Brillert (aragon) 2010-02-17 13:31:45.000-0600 wedhorn: Glad to see you are still around. I'm looking forward to seeing this issue move forward. I can help you test, but only version 1.4.x since I have no current plans to migrate to 1.6 or trunk anytime soon. By: Damien Wedhorn (wedhorn) 2010-02-17 13:36:12.000-0600 This patch does not fix any bugs (well it's not designed to) and so will probably only go to trunk (ie, it won't go to 1.4 or current 1.6 branches). By: David Brillert (aragon) 2010-02-17 13:47:11.000-0600 wedhorn: tilghman has led me to believe that this patch could fix my locking issues in ASTERISK-15611 What started out as that bug report led to a recommendation of a possible fix at https://reviewboard.asterisk.org/r/219/ but that patch was abandoned in favour of your code. My fingers are crossed, and hopefully I'm not heading down a bad path... it would be nice for a core developer to put in his extra 2 cents here. By: Damien Wedhorn (wedhorn) 2010-02-17 16:32:10.000-0600 aragon, looks like there is no way this is going to be applied to anything but trunk (see http://lists.digium.com/pipermail/asterisk-dev/2010-February/042425.html). The patch may take a bit longer as I'll look at integrating an atomic update of softhangup so we can remove all chan locking around readq changes. By: David Brillert (aragon) 2010-02-17 19:46:48.000-0600 wedhorn: that's good info. While I wait for your patch to go into trunk I hope something non invasive can be done by the developers to fix ASTERISK-15611 By: David Brillert (aragon) 2010-03-11 11:54:24.000-0600 Hi wedhorn:  Any success in moving this along? By: Damien Wedhorn (wedhorn) 2010-03-11 14:11:31.000-0600 No progress, we were looking at how to handle _softhangup. However, I'll work on this first and do _softhangup later, so there should be some movement soon. By: Damien Wedhorn (wedhorn) 2010-03-14 17:14:55 Created a branch (http://svnview.digium.com/svn/asterisk/team/wedhorn/readq-locking/) with patch updated to latest trunk. Changes viewable at http://svnview.digium.com/svn/asterisk?view=revision&revision=252359. By: David Brillert (aragon) 2011-01-06 15:11:30.000-0600 Has this work been abandoned? Revision 252359 Jump to revision: Author: wedhorn Date: Sun Mar 14 21:41:02 2010 UTC (9 months, 3 weeks ago) Changed paths: 5 Log Message: Merge old patch into current trunk. By: Leif Madsen (lmadsen) 2011-07-26 14:17:07.821-0500 Per the Asterisk maintenance timeline page at http://www.asterisk.org/asterisk-versions maintenance (bug) support for the 1.4 and 1.6.x branches has ended. For continued maintenance support please move to the 1.8 branch which is a long term support (LTS) branch. For more information about branch support, please see https://wiki.asterisk.org/wiki/display/AST/Asterisk+Versions If this is still an issue, please open a new issue so it can be re-triaged appropriately. Thanks!