546900 - Crash [@ nsURIHashKey::HashKey(nsIURI const*)]

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[[e-mail]]

This is on a build that I made 11 hours ago.

###!!! ASSERTION: Trying to unregister for a URI that wasn't registered!: 'Error', file History.cpp, line 285
###!!! ASSERTION: This should only fail if we misuse the API!: 'NS_SUCCEEDED(rv)', file Link.cpp, line 519
###!!! ASSERTION: Trying to unregister for a URI that wasn't registered!: 'Error', file History.cpp, line 285
###!!! ASSERTION: This should only fail if we misuse the API!: 'NS_SUCCEEDED(rv)', file Link.cpp, line 519
###!!! ASSERTION: Trying to unregister for a URI that wasn't registered!: 'Error', file History.cpp, line 285
###!!! ASSERTION: This should only fail if we misuse the API!: 'NS_SUCCEEDED(rv)', file Link.cpp, line 519
###!!! ASSERTION: mRegistered is true, but we have no cached URI?!: 'mCachedURI', file Link.cpp, line 514
###!!! ASSERTION: Must pass a non-null URI!: 'aURI', file History.cpp, line 279

places!nsURIHashKey::HashKey
places!nsTHashtable<mozilla::places::History::KeyClass>::s_HashKey
xpcom_core!PL_DHashTableOperate
places!nsTHashtable<mozilla::places::History::KeyClass>::GetEntry
places!mozilla::places::History::UnregisterVisitedCallback
gklayout!mozilla::dom::Link::UnregisterFromHistory
gklayout!mozilla::dom::Link::~Link
gklayout!nsHTMLAnchorElement::~nsHTMLAnchorElement
gklayout!nsHTMLAnchorElement::`scalar deleting destructor'
gklayout!nsNodeUtils::LastRelease
gklayout!nsGenericElement::Release
gklayout!nsHTMLAnchorElement::Release
xpcom_core!nsXPCOMCycleCollectionParticipant::Unroot
xpcom_core!nsCycleCollector::CollectWhite
xpcom_core!nsCycleCollector::FinishCollection
xpcom_core!nsCycleCollector_finishCollection
xpc3250!XPCCycleCollectGCCallback
mozjs!js_GC
mozjs!JS_GC
xpc3250!nsXPConnect::Collect
xpcom_core!nsCycleCollector::Collect
xpcom_core!nsCycleCollector_collect
gklayout!nsJSContext::CC
gklayout!nsJSContext::IntervalCC
gklayout!nsUserActivityObserver::Observe
xpcom_core!nsObserverList::NotifyObservers
xpcom_core!nsObserverService::NotifyObservers
gklayout!nsUITimerCallback::Notify
xpcom_core!nsTimerImpl::Fire
xpcom_core!nsTimerEvent::Run
xpcom_core!nsThread::ProcessNextEvent
xpcom_core!NS_ProcessNextEvent_P
gkwidget!nsBaseAppShell::Run
gkwidget!nsAppShell::Run
toolkitcomps!nsAppStartup::Run
xul!XRE_main
seamonkey!NS_internal_main
seamonkey!wmain
seamonkey!wmainCRTStartup
kernel32!BaseProcessStart

Comment 1

[[e-mail]]

Oh, and this is triggered by reading email with embedded mailto: links.

Comment 2

[[e-mail]]

OK, so I just got the assertion again on a page with a mailto: link.
(I had disabled the crash so I don't know whether it would have crashed.)

Comment 3

[Shawn Wilsher :sdwilsh]

The assertions make sense if we don't register, but things are strange.  We don't have any tests with mailto: links, so I guess we should add one...

Comment 4

[Ian Neal]

I also get this on linux, so changing -> All

Comment 5

[Ian Neal]

backtrace from linux if it helps:
#0  0x05f3047b in nsURIHashKey::HashKey (aKey=0x0) at ../../../../dist/include/nsURIHashKey.h:74
#1  0x00a4f605 in PL_DHashTableOperate (table=0xb3d529d8, key=0x80, op=<value optimized out>) at pldhash.c:615
#2  0x05f301a7 in nsTHashtable<mozilla::places::History::KeyClass>::GetEntry (this=0xb3d529d8, aKey=0x0) at ../../../../dist/include/nsTHashtable.h:170
#3  0x05f2f9d8 in mozilla::places::History::UnregisterVisitedCallback (this=0xb3d529d0, aURI=0x0, aLink=0xab37616c) at /mozdev/commsrc/mozilla/toolkit/components/places/src/History.cpp:283
#4  0x0570a92a in mozilla::dom::Link::UnregisterFromHistory (this=0xab37616c) at /mozdev/commsrc/mozilla/content/base/src/Link.cpp:518
#5  0x0570a9ed in mozilla::dom::Link::~Link (this=0xab37616c, __in_chrg=<value optimized out>) at /mozdev/commsrc/mozilla/content/base/src/Link.cpp:65
#6  0x05755c2c in nsHTMLAnchorElement::~nsHTMLAnchorElement (this=0xab376140, __in_chrg=<value optimized out>) at /mozdev/commsrc/mozilla/content/html/content/src/nsHTMLAnchorElement.cpp:146
#7  0x056def15 in nsNodeUtils::LastRelease (aNode=0xab376140) at /mozdev/commsrc/mozilla/content/base/src/nsNodeUtils.cpp:274
#8  0x056ce863 in nsGenericElement::Release (this=0xab376140) at /mozdev/commsrc/mozilla/content/base/src/nsGenericElement.cpp:4202
#9  0x0147ecc4 in DoDeferredRelease<nsISupports*> (array=@0xb7d8843c) at /mozdev/commsrc/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp:489
#10 0x0147f081 in XPCJSRuntime::GCCallback (cx=0xb3d9d400, status=JSGC_END) at /mozdev/commsrc/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp:760
#11 0x00d9a9c3 in jsds_GCCallbackProc (cx=0xb3d9d400, status=JSGC_END) at /mozdev/commsrc/mozilla/js/jsd/jsd_xpc.cpp:524
#12 0x05844186 in DOMGCCallback (cx=0xb3d9d400, status=JSGC_END) at /mozdev/commsrc/mozilla/dom/base/nsJSEnvironment.cpp:3735
#13 0x01466081 in XPCCycleCollectGCCallback (cx=0xb3d9d400, status=JSGC_END) at /mozdev/commsrc/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:413
#14 0x002580f8 in js_GC (cx=0xb3d9d400, gckind=GC_NORMAL) at /mozdev/commsrc/mozilla/js/src/jsgc.cpp:3383
#15 0x0022302a in JS_GC (cx=0xb3d9d400) at /mozdev/commsrc/mozilla/js/src/jsapi.cpp:2288
#16 0x0146593a in nsXPConnect::Collect (this=0xb7dd3190) at /mozdev/commsrc/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:479
#17 0x00aa5d29 in nsCycleCollector::Collect (this=0xb7d41800, aTryCollections=1) at /mozdev/commsrc/mozilla/xpcom/base/nsCycleCollector.cpp:2521
#18 0x00aa5de7 in nsCycleCollector_collect () at /mozdev/commsrc/mozilla/xpcom/base/nsCycleCollector.cpp:3216
#19 0x05844302 in nsJSContext::CC () at /mozdev/commsrc/mozilla/dom/base/nsJSEnvironment.cpp:3549
#20 0x0584436a in nsJSContext::IntervalCC () at /mozdev/commsrc/mozilla/dom/base/nsJSEnvironment.cpp:3637
#21 0x0584443d in nsJSContext::CCIfUserInactive () at /mozdev/commsrc/mozilla/dom/base/nsJSEnvironment.cpp:3625
#22 0x058445d1 in GCTimerFired (aTimer=0xab5914c0, aClosure=0x0) at /mozdev/commsrc/mozilla/dom/base/nsJSEnvironment.cpp:3663
#23 0x00a9a8a0 in nsTimerImpl::Fire (this=0xab5914c0) at /mozdev/commsrc/mozilla/xpcom/threads/nsTimerImpl.cpp:427
#24 0x00a9a97b in nsTimerEvent::Run (this=0xab588160) at /mozdev/commsrc/mozilla/xpcom/threads/nsTimerImpl.cpp:519
#25 0x00a96854 in nsThread::ProcessNextEvent (this=0xb7dea060, mayWait=1, result=0xbfffe82c) at /mozdev/commsrc/mozilla/xpcom/threads/nsThread.cpp:527
#26 0x00a57989 in NS_ProcessNextEvent_P (thread=0xbfffa34c, mayWait=1) at nsThreadUtils.cpp:250
#27 0x026b45d9 in nsBaseAppShell::Run (this=0xb7ae1560) at /mozdev/commsrc/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:177
#28 0x046b79f5 in nsAppStartup::Run (this=0xb5aee910) at /mozdev/commsrc/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:183
#29 0x0012f7f7 in XRE_main (argc=1, argv=0xbfffede4, aAppData=0xb7d05600) at /mozdev/commsrc/mozilla/toolkit/xre/nsAppRunner.cpp:3489
#30 0x080493dc in main (argc=1, argv=0xbfffede4) at /mozdev/commsrc/suite/app/nsSuiteApp.cpp:105

Comment 6

[Ian Neal]

I'm guessing something in Bug 461199 will have caused this?

Comment 7

[Shawn Wilsher :sdwilsh]

Probably.  If someone can give me a reduced testcase (bonus points if it is in mochitest form) I promise a quick fix!

Comment 8

[Ian Neal]

Steps to reproduce:
1) Select email message with embedded mailto: link in
2) Navigate to second email message (with or without embedded mailto: link in)
3) Seg fault!

So it seems to be the act of navigating away from an email message with an embedded mailto: link in that causes the crash.

I've not seen it happen on web pages with embedded mailto: links in.

Comment 9

[Ian Neal]

Attachment: Reduced testcase

Managed to get it to happen on a browser window too.
Steps to reproduce on SeaMonkey:
1) Start SM loading testcase.html as a command line argument
2) Close SM
3) Seg fault

Comment 10

[Robert Kaiser]

#1 topcrash for SeaMonkey trunk right now:
http://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&signature=nsURIHashKey%3A%3AHashKey%28nsIURI%20const%2A%29&version=SeaMonkey%3A2.1a1pre

Comment 11

[Zane U. Ji]

Another way to reproduce:
1. Visit https://bugzilla.mozilla.org/show_bug.cgi?id=546900
2. Quit SM.

The problem is that mCachedURI is set to nsnull at
http://hg.mozilla.org/mozilla-central/annotate/4e1b68ecf126/content/base/src/Link.cpp#l492
while leave mRegistered untouched. If mRegistered is true, SM crashes when Link::UnregisterFromHistory() is called again.

Comment 12

[Shawn Wilsher :sdwilsh]

(In reply to comment #11)
> The problem is that mCachedURI is set to nsnull at
> http://hg.mozilla.org/mozilla-central/annotate/4e1b68ecf126/content/base/src/Link.cpp#l492
> while leave mRegistered untouched. If mRegistered is true, SM crashes when
> Link::UnregisterFromHistory() is called again.
That's not true.  We touch mRegistered here in Link::UnregisterFromHistory.

Comment 13

[[e-mail]]

OK, so the problem seems to be a bug in nsMailtoUrl::Equals - it doesn't even compare equal to itself.

Comment 14

[Shawn Wilsher :sdwilsh]

(In reply to comment #13)
> OK, so the problem seems to be a bug in nsMailtoUrl::Equals - it doesn't even
> compare equal to itself.
That doesn't exist in mozilla-central, right?  This would also explain a bunch of the assertions.

Comment 15

[[e-mail]]

Attachment: Proposed patch

As this is a topcrash for SeaMonkey I would also appreciate approval to land on the restricted tree.

Comment 16

[Justin Wood (:Callek)]

Comment on attachment 429368 [details] [diff] [review]
Proposed patch

>+    return other->Equals(m_baseURL, _retval);
>+
> 	return m_baseURL->Equals(other, _retval);

Nit kill the tab while you are here.

Comment 17

[Zane U. Ji]

(In reply to comment #12)
> That's not true.  We touch mRegistered here in Link::UnregisterFromHistory.
Not if UnregisterVisitedCallback failed.

Comment 18

[Robert Kaiser]

As a note, I have reports from people who say applying the patch in here stopped crashes they saw quite frequently, even in the SeaMonkey browser.

Comment 19

[Shawn Wilsher :sdwilsh]

(In reply to comment #17)
> (In reply to comment #12)
> > That's not true.  We touch mRegistered here in Link::UnregisterFromHistory.
> Not if UnregisterVisitedCallback failed.
Yes, but it only fails when things go wrong (like in this case).  If we touched mRegistered, we wouldn't then be able to assert about it.  The problem here is that nsMailToUrl::Equals is buggy.

Comment 20

[Ian Neal]

(In reply to comment #18)
> As a note, I have reports from people who say applying the patch in here
> stopped crashes they saw quite frequently, even in the SeaMonkey browser.
Yeah, fixes it for me too.

Comment 21

[David :Bienvenu]

Comment on attachment 429368 [details] [diff] [review]
Proposed patch

can you remove the tab on the following line while you're here?

Should our other mailnews urls do something similar?

Comment 22

[[e-mail]]

Pushed changeset bc9b4537e1c3 to comm-central.