[Home]

Summary:ASTERISK-09654: dead lock
Reporter:shawn (welles)Labels:
Date Opened:2007-06-11 20:31:49Date Closed:2007-07-05 09:07:06
Priority:BlockerRegression?No
Status:Closed/CompleteComponents:Applications/app_chanspy
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) issue9951.diff
Description:in channel.c 's function ast_read use lock in order of chan then spy, while in app_chanspy.c's function channel_spy use lock in order of spy then chan. these order seems easy to cause dead lock when lots of spies(more than 100 spies spy on more than 4 channels). BTW, when i add spies more than 104, some of my clients begin hangup by asterisk. i am confused by this.
Comments:By: Joshua C. Colp (jcolp) 2007-06-12 08:42:16

This is a known issue... the spy architecture is flawed when it comes to locking. I've rewritten it mostly from scratch but I don't dare put that stuff into 1.2 or 1.4 as it is very invasive.
By: shawn (welles) 2007-06-15 01:52:03

dear file,
i simply change the following source code from
ast_mutex_lock(&csth.spy.lock);
if (csth.spy.chan) {
       csth.spy.status = CHANSPY_DONE;
ast_mutex_lock(&csth.spy.chan->lock);
ast_channel_spy_remove(csth.spy.chan, &csth.spy);
ast_mutex_unlock(&csth.spy.chan->lock);
}
ast_mutex_unlock(&csth.spy.lock);
to
if (csth.spy.chan) {
csth.spy.status = CHANSPY_DONE;
ast_mutex_lock(&csth.spy.chan->lock);
ast_mutex_lock(&csth.spy.lock);
ast_channel_spy_remove(csth.spy.chan, &csth.spy);
ast_mutex_unlock(&csth.spy.lock);
ast_mutex_unlock(&csth.spy.chan->lock);
}
dead lock disappear. is there anything unsuitable? how it will cause to be invasive?
any comments will be great appreciated.
By: Joshua C. Colp (jcolp) 2007-06-15 07:26:15

Now instead of deadlocking you've got the potential to crash because chan may be set to NULL after you've done the check to make sure it isn't, and before you lock it.
By: Joshua C. Colp (jcolp) 2007-06-18 18:16:18

Please try the attached patch. It tweaks locking a bit.
By: Digium Subversion (svnbot) 2007-07-05 09:02:36

Repository: asterisk
Revision: 73349

------------------------------------------------------------------------
r73349 | file | 2007-07-05 09:02:35 -0500 (Thu, 05 Jul 2007) | 2 lines

Tweak spy locking. (issue ASTERISK-9654 reported by welles)

------------------------------------------------------------------------
By: Digium Subversion (svnbot) 2007-07-05 09:05:07

Repository: asterisk
Revision: 73355

------------------------------------------------------------------------
r73355 | file | 2007-07-05 09:05:06 -0500 (Thu, 05 Jul 2007) | 10 lines

Merged revisions 73349 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r73349 | file | 2007-07-05 11:19:14 -0300 (Thu, 05 Jul 2007) | 2 lines

Tweak spy locking. (issue ASTERISK-9654 reported by welles)

........

------------------------------------------------------------------------
By: Digium Subversion (svnbot) 2007-07-05 09:06:21

Repository: asterisk
Revision: 73359

------------------------------------------------------------------------
r73359 | file | 2007-07-05 09:06:20 -0500 (Thu, 05 Jul 2007) | 18 lines

Merged revisions 73355 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

................
r73355 | file | 2007-07-05 11:21:44 -0300 (Thu, 05 Jul 2007) | 10 lines

Merged revisions 73349 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r73349 | file | 2007-07-05 11:19:14 -0300 (Thu, 05 Jul 2007) | 2 lines

Tweak spy locking. (issue ASTERISK-9654 reported by welles)

........

................

------------------------------------------------------------------------
By: Joshua C. Colp (jcolp) 2007-07-05 09:07:05

I've put this patch into 1.2 as of revision 73349, 1.4 as of revision 73355, and trunk as of revision 73359.