Last Comment Bug 696913 - deadlock in nsImapProtocol::SendData if UI thread tries to check CanHandleUrl()
: deadlock in nsImapProtocol::SendData if UI thread tries to check CanHandleUrl()
Status: RESOLVED FIXED
: hang
Product: MailNews Core
Classification: Components
Component: Networking: IMAP (show other bugs)
: unspecified
: x86_64 Windows 7
: -- critical (vote)
: Thunderbird 11.0
Assigned To: David :Bienvenu
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-24 15:16 PDT by David :Bienvenu
Modified: 2011-11-15 07:24 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (1.40 KB, patch)
2011-10-25 17:47 PDT, David :Bienvenu
irving: review+
Details | Diff | Splinter Review

Description David :Bienvenu 2011-10-24 15:16:06 PDT
I ran into a deadlock with uploading a message to an imap folder - nsImapProtocol::SendData enter a monitor on the protocol object

PR_CEnterMonitor(this)

before writing data to the output stream.

Then, the UI thread comes along and tries to see if the protocol object can handle a URL, CanHandleUrl calls nsImapProtocol::IsBusy, which calls   NS_LOCK_INSTANCE();, which is PR_CEnterMonitor(this), which is deadlock. Not quite sure why the SendData call is blocking on something happening on the UI thread, but it is. I wonder if fixing nsSyncRunnable not to use the event queue might help...

if not, IsBusy probably doesn't have to ener the monitor, or, SendData could use a separate monitor for the output stream.
Comment 1 :Irving Reid (No longer working on Firefox) 2011-10-25 09:19:28 PDT
David's reproduction suggestions - I'm trying them now...

09:51 (bienvenu) possible steps would be -
09:51 (bienvenu) set the number of cached connections to 1 or 2 in advanced imap server settings (might be easier to see if you do this, not sure)
09:51 (bienvenu) copy a large local message up to an imap server
09:52 (bienvenu) while that's going on, click on get new mail, or somehow cause an imap url to get run
09:52 (bienvenu) then figure out if comm-beta or aurora has the same bug
09:52 (bienvenu) if so, then it's not the sync runnable changes
09:54 (bienvenu) if not, then the fix to try would be to do what I suggest in https://bugzilla.mozilla.org/show_bug.cgi?id=675407#c50
09:54 (firebot) Bug 675407 nor, --, Thunderbird 10.0, dbienvenu, RESO FIXED, Remove XPCOM proxies from comm-central
Comment 2 David :Bienvenu 2011-10-25 13:30:06 PDT
this happens in comm-beta as well, so it's not caused by the xpcom proxy removal.
Comment 3 David :Bienvenu 2011-10-25 17:47:12 PDT
Created attachment 569569 [details] [diff] [review]
proposed fix

this fixes the hang for me, which I can fairly easily reproduce on Windows. The code that touches m_urlInProgress from the imap thread already has the protocol's mLock. The other code that touches m_urlInProgress is run on the UI thread, so there won't be a race there.
Comment 4 Ludovic Hirlimann [:Usul] 2011-11-09 08:42:16 PST
David does this needs commiting ?
Comment 5 David :Bienvenu 2011-11-09 08:54:48 PST
yeah, I'll land it soon.
Comment 6 David :Bienvenu 2011-11-15 07:24:10 PST
fixed on trunk - http://hg.mozilla.org/comm-central/rev/a17719df870c

Note You need to log in before you can comment on or make changes to this bug.