Closed Bug 1042294 Opened 5 years ago Closed 5 years ago

crash NS_ProxyRelease(nsIEventTarget*, nsISupports*, bool), typically during import

Categories

(MailNews Core :: Import, defect, critical)

defect
Not set
critical

Tracking

(thunderbird32 fixed, thunderbird33 fixed, thunderbird34 fixed, thunderbird_esr3132+ fixed)

VERIFIED FIXED
Thunderbird 34.0
Tracking Status
thunderbird32 --- fixed
thunderbird33 --- fixed
thunderbird34 --- fixed
thunderbird_esr31 32+ fixed

People

(Reporter: wsmwk, Assigned: hiro)

References

()

Details

(Keywords: crash, regression, topcrash-thunderbird, Whiteboard: [regression:TB29])

Crash Data

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #945045 +++

I first noted this crash on May 1. In betas it has been #3 crash. 
Today it is #1 crash for TB31.0. 
Crash rate is double the #2 ranked crash. 

First observed in TB29 beta, which includes the patch for bug 945045.
It's unclear to me why the average user would be crashing here.
I am waiting to hear back from some users

nsProxySendRunnable::Run() user is p.zan
bp-f5a9e1a1-e466-4aae-979b-746692140718
0	xul.dll	nsProxySendRunnable::Run()	mailnews/import/src/nsImportService.cpp
1	xul.dll	nsThread::ProcessNextEvent(bool,bool *)	xpcom/threads/nsThread.cpp
2	xul.dll	NS_ProcessNextEvent(nsIThread *,bool)	xpcom/glue/nsThreadUtils.cpp
3	xul.dll	nsXULWindow::ShowModal()	xpfe/appshell/src/nsXULWindow.cpp
4	xul.dll	nsContentTreeOwner::ShowAsModal()	xpfe/appshell/src/nsContentTreeOwner.cpp
5	xul.dll	nsWindowWatcher::OpenWindowInternal(nsIDOMWindow *,char const *,char const *,char const *,bool,bool,bool,nsIArray *,nsIDOMWindow * *)	embedding/components/windowwatcher/src/nsWindowWatcher.cpp
6	xul.dll	nsWindowWatcher::OpenWindow2(nsIDOMWindow *,char const *,char const *,char const *,bool,bool,bool,nsISupports *,nsIDOMWindow * *)	embedding/components/windowwatcher/src/nsWindowWatcher.cpp
7	xul.dll	nsGlobalWindow::OpenInternal(nsAString_internal const &,nsAString_internal const &,nsAString_internal const &,bool,bool,bool,bool,bool,nsIArray *,nsISupports *,nsIPrincipal *,JSContext *,nsIDOMWindow * *)	dom/base/nsGlobalWindow.cpp
8	xul.dll	nsGlobalWindow::OpenInternal(nsAString_internal const &,nsAString_internal const &,nsAString_internal const &,bool,bool,bool,bool,bool,nsIArray *,nsISupports *,nsIPrincipal *,JSContext *,nsIDOMWindow * *)	dom/base/nsGlobalWindow.cpp
9	xul.dll	nsGlobalWindow::OpenDialog(JSContext *,nsAString_internal const &,nsAString_internal const &,nsAString_internal const &,mozilla::dom::Sequence<JS::Value> const &,mozilla::ErrorResult &)	dom/base/nsGlobalWindow.cpp
10	xul.dll	mozilla::dom::WindowBinding::openDialog	objdir-tb/mozilla/dom/bindings/WindowBinding.cpp
11	xul.dll	mozilla::dom::WindowBinding::genericMethod	objdir-tb/mozilla/dom/bindings/WindowBinding.cpp 

p.zan also crashed  NS_ProxyRelease(nsIEventTarget*, nsISupports*, bool) bp-6f0cf35b-0eda-4e12-8976-a8e722140721 which is bug 902158.  But I none of the other users I checked had that correlation. For the most part those that crashed multiple times had no other crash sigs.


bp-f5a9e1a1-e466-4aae-979b-746692140718
http://hg.mozilla.org/releases/mozilla-beta/annotate/6687aa144412/xpcom/threads/nsThread.cpp#l715
bzbarsky@29077 711 if (event) {
bzbarsky@29077 712   LOG(("THRD(%p) running [%p]\n", this, event.get()));
benjamin@82283 713   if (MAIN_THREAD == mIsMainThread)
benjamin@82283 714     HangMonitor::NotifyActivity();
bzbarsky@29077 715   event->Run(); 

http://hg.mozilla.org/releases/comm-beta/annotate/4ab5e7d55ce4/mailnews/import/src/nsImportService.cpp#l341
bienvenu@9140 331 NS_IMETHODIMP nsProxySendRunnable::Run()
bienvenu@9140 332 {
bienvenu@9140 333 nsresult rv;
bienvenu@9140 334 nsCOMPtr<nsIMsgSend> msgSend = do_CreateInstance(NS_MSGSEND_CONTRACTID, &rv);
bienvenu@9140 335 NS_ENSURE_SUCCESS(rv, rv);
bienvenu@9140 336
hiikezoe@17258 337 nsCOMPtr<nsISupportsArray> supportsArray;
hiikezoe@17258 338 NS_NewISupportsArray(getter_AddRefs(supportsArray));
hiikezoe@17258 339
hiikezoe@17258 340 nsCOMPtr<nsISimpleEnumerator> enumerator;
hiikezoe@17258 341 m_embeddedAttachments->Enumerate(getter_AddRefs(enumerator));
Attached patch FixSplinter Review
That's my mistake.

m_embeddedAttachments is null in case of no attachment files. We have to care the case.
Attachment #8461307 - Flags: review?(standard8)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)
> Created attachment 8461307 [details] [diff] [review]
> Fix
> 
> That's my mistake.
> 
> m_embeddedAttachments is null in case of no attachment files. We have to
> care the case.

Hiro-san could you also try to write a test ?
(In reply to Ludovic Hirlimann [:Usul] from comment #2)

> Hiro-san could you also try to write a test ?

Yes. I will take time to write a test on this weekend.
Attachment #8461307 - Flags: review?(standard8) → review+
Summary: crash NS_ProxyRelease(nsIEventTarget*, nsISupports*, bool) → crash NS_ProxyRelease(nsIEventTarget*, nsISupports*, bool), typically during import
> status-thunderbird_esr31: --- → fixed
Mark, we don't have a checkin comment. Did you intend to set status to "affected"?
Flags: needinfo?(standard8)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)
> (In reply to Ludovic Hirlimann [:Usul] from comment #2)
> 
> > Hiro-san could you also try to write a test ?
> 
> Yes. I will take time to write a test on this weekend.

hiro, per someone (standard8 or irving via IRC iirc) fix can land without test patch.  So please land fix, and test patch can come later.
Flags: needinfo?(hiikezoe)
I have written the test but can not test it yet because I can't build comm-central on Windows.
So the test still be incomplete.
Flags: needinfo?(hiikezoe)
Yes please land without test, I will open a new bug for the test.
Keywords: checkin-needed
Comment on attachment 8461307 [details] [diff] [review]
Fix

[Triage Comment]
I'll do the trunk landing in a while..., lets also get this landed in aurora and beta for additional testing, especially as we haven't got full coverage on trunk at the moment.
Attachment #8461307 - Flags: approval-comm-beta+
Attachment #8461307 - Flags: approval-comm-aurora+
https://hg.mozilla.org/comm-central/rev/320e197c11d5
https://hg.mozilla.org/releases/comm-aurora/rev/0bfaaa91434f
https://hg.mozilla.org/releases/comm-beta/rev/30bbf5ac51f4

I'll mark this as fixed since the main patch has landed.
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 34.0
Comment on attachment 8461307 [details] [diff] [review]
Fix

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky):
Attachment #8461307 - Flags: approval-comm-esr31?
Attachment #8461307 - Flags: approval-comm-esr31? → approval-comm-esr31+
Depends on: 1061086
You need to log in before you can comment on or make changes to this bug.