Convert IMAP protocol to NS_DispatchBackgroundTask
Categories
(MailNews Core :: Networking: IMAP, task, P3)
Tracking
(Not tracked)
People
(Reporter: rjl, Assigned: benc)
References
(Blocks 1 open bug, Regression)
Details
Attachments
(2 obsolete files)
Bug 1613440 deprecates use of NS_NewNamedThread. The suggest migration is to use NS_DispatchBackgroundTask or NS_CreateBackgroundTaskQueue.
More details and some examples can be found in this PSA:
https://groups.google.com/forum/#!msg/mozilla.dev.platform/SwmYPy_8_8k/2vswXjUpBgAJ
In the meantime, the "IMAP" thread name is being added to ThreadAllow.txt.
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
Been picking my way through this one today - just wanted to jot down some notes to clarify my thinking on it so far.
It's a little more involved than the libical one as the thread stays in a long-running loop, rather than just kicking off one-shot jobs. We don't want to tie up the background thread(s) with a loop.
The loop essentially just waits until ready for the the next url, so NS_CreateBackgroundTaskQueue
should be a pretty natural replacement - there are a couple of places which signal the thread to check for the next url, so instead they'll just submit new background tasks directly.
There's a little complication (imap IDLE is also handled in the loop), but that's my general plan so far.
(I suspect doing this might also help shed some light on Bug 1586494 - even just by virtue of emerging with a better understanding of the IMAP threading..)
Updated•5 years ago
|
Comment 3•5 years ago
|
||
Sounds promising.
Given that Magnus marked bug 1586494 P1, I'm setting that here as well. If that is not accurate feel free to change.
Assignee | ||
Comment 4•5 years ago
|
||
Been digging through the IMAP code, so here's a stab at trying to flesh out the comments of various classes and interfaces as I go along. Likely to be more of these.
Comment 5•5 years ago
|
||
Assignee | ||
Comment 6•5 years ago
|
||
Comment 7•5 years ago
|
||
Assignee | ||
Comment 8•5 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #7)
Comment on attachment 9133729 [details] [diff] [review]
::: mailnews/imap/src/nsImapProtocol.h
- /** The currently-running request. */
Hmm, there is no hyphen between the adverb and the gerund.
Gah, yes - a grammatical tick of mine!
Fixed up and moved over to Bug 1622979.
Assignee | ||
Updated•5 years ago
|
Comment 9•4 years ago
|
||
Ben, would it be helpful to have this patched before making more intrusive/risky changes that are needed to resolve our various longstanding imap threading bugs?
Assignee | ||
Comment 10•4 years ago
|
||
(In reply to Wayne Mery (:wsmwk) from comment #9)
Ben, would it be helpful to have this patched before making more intrusive/risky changes that are needed to resolve our various longstanding imap threading bugs?
I don't know for sure, but I think it won't make any difference. Whatever happens in this bug, we're still running all the IMAP protocol code on a non-main thread, so all the same sync issues remain.
I've a suspicion that my thoughts at the time of comment #2 were a bit naive. The approach still sounds feasible, but it troubles me that we'd be going from a nice simple single-blockable-thread-of-execution model a to more complicated dispatch system, but without going to the full state-machine approach that the IMAP thread was aiming to avoid. So, more complexity. And it's not like we don't already have enough complexity in IMAP :-). But I think it's just going to be a matter of having a go at at, and being prepared to chuck it away if it adds too much hassle.
Comment 11•4 years ago
|
||
Right, I wasn't suggesting this will in any way fix any aspect of imap sync. Rather I was curious if it might make help development/debugging of the imap fixes, and then after shipping help with the inevitable issues that will come.
Parallel ideas:
- we don't know when they will actually remove the current code, which could force to adopt this at an inconvenient time in our own development process - there's nothing imminent in any of the blocking bugs, but that can always change. And as you stated our threading is complex, so we wouldn't want to risk being forced to land this late in an esr development cycle.
- as you indicate this may complicate fixing the imap sync problems, so is it preferable to do these at the same time as one package, or do them at different times, and if so in which order?
Updated•4 years ago
|
Updated•2 years ago
|
Description
•