431460 - Imap protocol module needs design and implementation improvements to minimize cross-threading issues

Status: RESOLVED WORKSFORME

(MailNews Core :: Networking: IMAP, defect)

Description

[Emre Birol]

Currently, imap protocol implementation executes its own internal thread to handle the imap session with the server. Some methods of nsImapProtocol class are meant to be called by this thread, and some of them by UI thread. This is not so obvious by the current design/implementation. This makes the code error prone (the consequences of not using the right thread usually is a crash or a deadlock), hard to understand and hard to maintain.

nsImapProtocol should be carefully re-factored in order to guarantee that
1- objects are de-initialized by their owner threads (there are couple assertions should be addressed) 
2- threads do not make cross-boundary calls
   i.e. Off UI threads should not call UI components directly
3- threads do not cause deadlocks or race conditions
   i.e. RetyUrl is one area that needs attention

Additionally, decoupling imap parser from the generic imap session management
could allow us to test the parser separately from the rest of the system.  

Current design can be viewed at:
http://wiki.mozilla.org/User:Emre/tb/architecture/diagrams

(See also notes at bug 410747)

Comment 1

[Emre Birol]

In support of item #2; see Bug 383489 

Comment 2

[David :Bienvenu]

We've added some assertions when methods are called from the wrong thread, and documented which nsImapProtocol methods can be called from the UI thread, which are the exception - almost all are only called from the imap thread.