12870 - AB-BA deadlocks between pipe and channel critical sections

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Brendan Eich [:brendan]]

Nested monitors should be avoided if possible, minimizing critical sections to a
few instructions.  If you can't do that, then you must define a canonical order
for nesting and stick to it.  Can I help get this straight?  It's blocking lots
of folks, and although I got past it on my first apprunner session of the day,
ever since then apprunner deadlocks starting up.

/be

Necko thread has filechannel monitor, is trying to get pipe monitor:

PR_Lock(PRLock * 0x009e1030) line 220
PR_EnterMonitor(PRMonitor * 0x009e10e0) line 79 + 14 bytes
PR_CEnterMonitor(void * 0x02273910) line 314 + 9 bytes
nsAutoCMonitor::nsAutoCMonitor(void * 0x02273910) line 198 + 11 bytes
nsPipe::nsPipeOutputStream::WriteSegments(nsPipe::nsPipeOutputStream * const
0x02273924,
unsigned int (void *, char *, unsigned int, unsigned int, unsigned int *)*
0x10031b40
nsReadFromInputStream(void *, char *, unsigned int, unsigned int, unsigned int
*), void *
0x022730b8, unsigned int 0x00002da7, unsigned int * 0x0134fefc) line 599
nsPipe::nsPipeOutputStream::WriteFrom(nsPipe::nsPipeOutputStream * const
0x02273924,
nsIInputStream * 0x022730b8, unsigned int 0x00002da7, unsigned int * 0x0134fefc)
line 739
nsFileChannel::Process() line 665 + 30 bytes
nsFileChannel::Run(nsFileChannel * const 0x02273764) line 591
nsThreadPoolRunnable::Run(nsThreadPoolRunnable * const 0x00ac8a40) line 563 + 12
bytes
nsThread::Main(void * 0x00ac8a00) line 102 + 18 bytes
_PR_NativeRunThread(void * 0x00ac8890) line 379 + 13 bytes
_threadstartex(void * 0x00ac86e0) line 212 + 13 bytes

Mozilla thread has pipe monitor, is trying to get filechannel monitor:

PR_Lock(PRLock * 0x022735d0) line 220
PR_EnterMonitor(PRMonitor * 0x02273720) line 79 + 14 bytes
nsAutoMonitor::nsAutoMonitor(PRMonitor * 0x02273720) line 139 + 11 bytes
nsFileChannel::Resume(nsFileChannel * const 0x02273760) line 267
nsFileChannel::OnEmpty(nsFileChannel * const 0x02273768, nsIPipe * 0x02273910)
line 796
nsPipe::nsPipeInputStream::ReadSegments(nsPipe::nsPipeInputStream * const
0x02273918,
unsigned int (void *, const char *, unsigned int, unsigned int, unsigned int *)*
0x100312d0 nsWriteToRawBuffer(void *, const char *, unsigned int, unsigned int,
unsigned
int *), void * 0x00908be8, unsigned int 0x00000400, unsigned int * 0x006af768)
line 363 +
22 bytes
nsPipe::nsPipeInputStream::Read(nsPipe::nsPipeInputStream * const 0x02273918,
char *
0x00908be8, unsigned int 0x00000400, unsigned int * 0x006af768) line 462
ByteBufferImpl::Fill(ByteBufferImpl * const 0x02272e30, unsigned int *
0x006af7a8,
nsIInputStream * 0x02273918, unsigned int 0x00000000) line 126 + 36 bytes
ConverterInputStream::Fill(unsigned int * 0x006af7a8) line 269 + 33 bytes
ConverterInputStream::Read(ConverterInputStream * const 0x022732a0, unsigned
short *
0x009292ec, unsigned int 0x00000000, unsigned int 0x00000400, unsigned int *
0x006af7dc)
line 242 + 12 bytes
nsExpatTokenizer::LoadStream(nsIInputStream * 0x02273918, unsigned short * &
0x00000000,
unsigned int & 0x00000000) line 562 + 30 bytes
nsExpatTokenizer::HandleExternalEntityRef(void * 0x02271290, const char *
0x00000000,
const char * 0x009269f0, const char * 0x00926aa6, const char * 0x00000000) line
622 + 23
bytes
doProlog(void * 0x02271290, const encoding * 0x00f53708 little2_encoding, const
char *
0x009373ec, const char * 0x00946a1c, int 0x00000011, const char * 0x009373ee,
const char *
* 0x006af994) line 2272 + 36 bytes
prologProcessor(void * 0x02271290, const char * 0x00936a1c, const char *
0x00946a1c, const
char * * 0x006af994) line 2145 + 36 bytes
prologInitProcessor(void * 0x02271290, const char * 0x00936a1c, const char *
0x00946a1c,
const char * * 0x006af994) line 2134 + 21 bytes
XML_Parse(void * 0x02271290, const char * 0x00936a1c, int 0x00010000, int
0x00000000) line
867 + 40 bytes
nsExpatTokenizer::ParseXMLBuffer(const char * 0x00936a1c, unsigned int
0x00010000, int
0x00000000) line 286 + 24 bytes
nsExpatTokenizer::ConsumeToken(nsScanner & {...}) line 329 + 18 bytes
nsParser::Tokenize(int 0x00000000) line 1268 + 21 bytes
nsParser::ResumeParse(nsIDTD * 0x00000000, int 0x00000000) line 885 + 12 bytes
nsParser::OnDataAvailable(nsParser * const 0x0222a144, nsIChannel * 0x020f2760,
nsISupports * 0x00000000, nsIInputStream * 0x020f23a8, unsigned int 0x00000000,
unsigned
int 0x00008000) line 1161 + 19 bytes
nsDocumentBindInfo::OnDataAvailable(nsDocumentBindInfo * const 0x020f1180,
nsIChannel *
0x020f2760, nsISupports * 0x00000000, nsIInputStream * 0x020f23a8, unsigned int
0x00000000, unsigned int 0x00008000) line 2205 + 32 bytes
nsChannelListener::OnDataAvailable(nsChannelListener * const 0x020f2e60,
nsIChannel *
0x020f2760, nsISupports * 0x00000000, nsIInputStream * 0x020f23a8, unsigned int
0x00000000, unsigned int 0x00008000) line 2469
nsFileChannel::OnDataAvailable(nsFileChannel * const 0x020f276c, nsIChannel *
0x020f2760,
nsISupports * 0x00000000, nsIInputStream * 0x020f23a8, unsigned int 0x00000000,
unsigned
int 0x00008000) line 810
nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const 0x020f3fc0)
line 345
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x020f3fc4) line 144 + 12 bytes
PL_HandleEvent(PLEvent * 0x020f3fc4) line 509 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00ac5ce0) line 470 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00001428, unsigned int 0x0000d2b1, unsigned int
0x00000000, long 0x00ac5ce0) line 938 + 9 bytes

Comment 1

[Warren Harris]

Checked in fix.

Comment 2

[Paul MacQuiddy]

brendan, if you agree with fix, please mark verified, this is over my head

Comment 3

[brendan-obsolete]

I'm working on DEBUG-only deadlock detection in nsAutoLock.h, so I'll VERIFY
this once that's working and checked-in.

/be

Comment 4

[leger]

Bulk move of all Necko (to be deleted component) bugs to new Networking

component.

Comment 5

[benc]

brendan: any chance you could VERIFY this bug? It is over my head as well :)

Comment 6

[Brendan Eich [:brendan]]

Wow, is this old.  I'll verify it just cuz it must have been fixed, or we'd have
heard more.  Cc'ing darin for history's sake.

/be

Comment 7

[Darin Fisher]

yeah, a lot of the code in the stack trace (especially that having to do with
nsFileChannel) has been totally reworked.