1649188 - Race condition on Thunderbird initial startup when opening a tab

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[[e-mail]]

Steps to reproduce problem:

1. Compile Thunderbird 78 with a custom source for Owl, so that it picks up a version that actually runs in Thunderbird 78
2. Start a new profile
3. Set up an account which requires Owl to log in using a browser
4. Complete account setup
5. Dismiss the default mail dialog

Expected results: After account setup is complete, Owl logs in to the account in a tab, and discovers folders

Actual results: Exception thrown in calendar code.

What happens here is that the default mail dialog is modal, which means that Thunderbird's onload handler has not yet completed, and therefore calendar's main window onload handler has not even started to execute. However, Owl is trying to authenticate in the background by opening a browser tab. This action causes Calendar to try to update its UI, which fails because its onload handler has not yet executed.

Comment 1

[[e-mail]]

Attachment: Catch errors in listeners

Actually any extension can trigger the bug by opening a tab while the default client dialog is open.

Comment 2

[Ben Bucksch (:BenB)]

Even though there's Owl appearing in the reproduction steps, the bug here is not in Owl. Owl is merely opening a tab, nothing more. That mere opening of the tab is completely breaking Thunderbird. That shouldn't happen.

This is the right fix for the problem.

The concept of observers is to decouple modules. An observer, from the perspective of the main module, is by definition always optional. An observer should never ever be able to break other, later observers, nor the main module. Imagine if a broken event listener would break the entire browser. Likewise, an optional module like the calendar should not break the mail reading.

Firefox in its <tabbrowser> implementation does the same thing, it's wrapping the call to the listeners in a try/catch. tabmail should do the same.

https://searchfox.org/mozilla-central/source/browser/base/content/tabbrowser.js#814 function _callListeners()

Comment 3

[Geoff Lankow (:darktrojan)]

Comment on attachment 9163033 [details] [diff] [review]
Catch errors in listeners

Seems reasonable. I hate tabmail. :-)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b2fc6b0388b8
Wrap calls to tab monitors in try/catch to prevent them from breaking tabmail. r=darktrojan DONTBUILD

Comment 5

[Ben Bucksch (:BenB)]

Comment on attachment 9163033 [details] [diff] [review]
Catch errors in listeners

[Approval Request Comment]
Regression caused by (bug #): tabmail XBL rewrite
User impact if declined:
Any error in any tab observer of any kind, anywhere in the app, even in optional components, can completely break the Thunderbird app and the tabs and the main window.
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky):
Just adding a try/catch. Very safe.

Comment 6

[Wayne Mery (:wsmwk)]

Comment on attachment 9163033 [details] [diff] [review]
Catch errors in listeners

Approved for esr78

Comment 7

[Ben Bucksch (:BenB)]

Thank you, Neil, Geoff and Wayne!

Comment 8

[Rob Lemley [:rjl]]

Comment on attachment 9163033 [details] [diff] [review]
Catch errors in listeners

[Triage Comment]
Taking for 79.0b2 for consistency.

Comment 9

[Rob Lemley [:rjl]]

Thunderbird 79.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/5bbe33b8430f34f916db9e89151f52f8755f785e

Comment 10

[Rob Lemley [:rjl]]

Thunderbird 78.0.1:
https://hg.mozilla.org/releases/comm-esr78/rev/bdbecbab1b92