Closed Bug 1408722 Opened 3 years ago Closed 3 years ago

Fix sync runnables to return NS_OK and pass result some other way

Categories

(MailNews Core :: Backend, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 58.0

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

Details

Attachments

(1 file, 1 obsolete file)

While fixing bug 1176748 I noticed that sync runnables must return NS_OK or else the caller will assert here:
https://dxr.mozilla.org/mozilla-central/rev/a31334a65a1c75638efae4452ecd271450df2ad0/xpcom/threads/nsThreadSyncDispatch.h#40
In any case, even in an opt build the error is silently ignored, hey, no one noticed so far :-(

We use sync runnables in four files:
https://dxr.mozilla.org/comm-central/search?q=NS_DISPATCH_SYNC&redirect=false
nsImapProtocol.cpp, nsBeckyMail.cpp, nsImportMail.cpp, mimecms.cpp.

I'm fixing nsImportMail.cpp in bug 1176748 and the one in nsImapProtocol.cpp is already OK. That leaved Becky import and Mime CMS.

Patch coming.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8918616 - Flags: review?(rkent)
Attachment #8918616 - Flags: review?(acelists)
Comment on attachment 8918616 [details] [diff] [review]
1408722-sync-runnables.patch (v1)

Review of attachment 8918616 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, looks reasonable.
Attachment #8918616 - Flags: review?(acelists) → review+
Oops, status of NS_DispatchToMainThread() should be checked, too.
Attachment #8918616 - Attachment is obsolete: true
Attachment #8918616 - Flags: review?(rkent)
Attachment #8918670 - Flags: review+
Here's an example showing "my method" used in M-C, albeit not with a failure result but something else:
http://searchfox.org/mozilla-central/rev/01970ed92d74f82d4e94a1e4365892bbcc593889/widget/windows/WidgetTraceEvent.cpp#79
  NS_DispatchToMainThread(getter, NS_DISPATCH_SYNC);
  return getter->hidden_window_hwnd;
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/af57e5297b94
Fix sync runnables to return NS_OK and pass result differently. r=aceman
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 58.0
Version: 52 → Trunk
You need to log in before you can comment on or make changes to this bug.