This covers what remains after Bug #1175168.

nsXPCWrappedJS::AddRef is #39 crash for 91.2.1, and #35 for 91.3.0.

Examples:

• bp-5705411b-c580-466f-a8b6-c356d0211109 91.2.1
  0 xul.dll nsXPCWrappedJS::AddRef() js/xpconnect/src/XPCWrappedJS.cpp:240
  1 xul.dll AddRefThunk(IUnknown*) ipc/mscom/VTableBuilder.c:22
  2 xul.dll nsMsgMailNewsUrl::~nsMsgMailNewsUrl() comm/mailnews/base/src/nsMsgMailNewsUrl.cpp:73
  3 xul.dll [thunk]: virtual void* __thiscall nsImapUrl::vector deleting destructor
  4 xul.dll nsMsgMailNewsUrl::Release comm/mailnews/base/src/nsMsgMailNewsUrl.cpp:81
  5 xul.dll nsImapUrl::Release() comm/mailnews/imap/src/nsImapUrl.cpp:81
  6 xul.dll nsImapProtocol::~nsImapProtocol() comm/mailnews/imap/src/nsImapProtocol.cpp:669
  7 xul.dll [thunk]: virtual void* __thiscall nsImapProtocol::`vector deleting destructor'(unsigned int)
  8 xul.dll mozilla::net::HttpBaseChannel::Release() netwerk/protocol/http/HttpBaseChannel.cpp:383

• bp-713462b1-ba1c-4e9c-b05f-440260211109 91.3.0 same stack

• bp-c5867ab5-09e8-46bb-9ac1-0c23e0211109 91.3.0 same stack

6 xul.dll nsImapProtocol::~nsImapProtocol() comm/mailnews/imap/src/nsImapProtocol.cpp:669

This is known issue for me. This is race condition of nsImapProtocol's release code. I will file new issue to fix this.

If those stacks are all the same, maybe m_kato's bug will take care of them all? (Or feel free to use this bug if you didn't file one already.)

I will post new patch next week.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=06a79e39def282ff1167e04ce3e321ff21e0d3cb

nsImapProtocol::Run has a race condition between main thread and IMAP thread.

IMAP thread is running Runnable by nsImapProtocol. But it is possible to run
releaseOnMain for nsIImapProtocol before decrementing reference count of
Runnable by nsImapProtocol on IMAP thread. So then, the destructor of
nsImapProtocol may run on IMAP thread that is shutting down.

In other words, this sequence is the following.

1. nsImapProtocol::Run() calls NS_ReleaseOnMainThread to dereference
   nsImapProtocol on Imap thread.
2. nsImapProtocol is released in main thread since we used
   NS_ReleaseOnMainThread.
3. nsThread::ProcessNextEvent releases nsImapProtocol on Imap thread since
   nsIRunnble is done and nsImapProtocol inherits nsIRunnable.
4. nsImapProtocol's reference is nothing, then it is deleted on Imap thread.

So nsImapProtocol shouldn't inherit nsIRunnable.

Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/comm-central/rev/2f0f28b861b6
nsImapProtocol should not inherit nsIRunnable. r=mkmelin

Thanks for the patch.

Doesn't need to ride the train if safe enough for uplift to beta. What do you think, OK to take next week on Monday's beta?

(In reply to Wayne Mery (:wsmwk) from comment #7)

Thanks for the patch.

Doesn't need to ride the train if safe enough for uplift to beta. What do you think, OK to take next week on Monday's beta?

Risk is medium since this fix changes object (nsImapProtcol)'s lifetime. But this can fix a lot of crashes, so I think that this should be uplifted.

Comment on attachment 9253842 [details]
Bug 1742991 - nsImapProtocol should not inherit nsIRunnable. r=mkmelin

[Triage Comment]
Approved for beta (it's been on nightly for a couple weeks)

We'll just want to keep a close eye out for regression behavior.

Thunderbird 96.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/5dfcfe8d7e57

How do you feel about uplifting this to version 91? If so, it would likely go into 91.5.1 shipping in a week.

It's been on nightly for 6 weeks and beta for 4 weeks. So far, I'm not seeing any regression bug reports.

(In reply to Wayne Mery (:wsmwk) from comment #11)

How do you feel about uplifting this to version 91? If so, it would likely go into 91.5.1 shipping in a week.

It's been on nightly for 6 weeks and beta for 4 weeks. So far, I'm not seeing any regression bug reports.

If no regression, I think that this should uplift to 91 since this is crash issue by assertion.

Comment on attachment 9253842 [details]
Bug 1742991 - nsImapProtocol should not inherit nsIRunnable. r=mkmelin

[Triage Comment]
Approved for esr91

Thunderbird 91.5.1:
https://hg.mozilla.org/releases/comm-esr91/rev/edd05aa1fff7

m_kato, thanks for your hard work!

Unfortunately I didn't analyze the results when this landed in version 91 almost exactly a year ago, and we don't have crash statistics going back that far. So we don't have a record of which dependent bugs improved. Still open are bug 558452, bug 917961, bug 1344036, bug 1508649 a topcrash, bug 1581079, bug 1746655.

FWIW nsXPCWrappedJS::AddRef itself was a low crash rate as of August

I filed a related issue as bug 1813422