Closed Bug 1655627 Opened 2 years ago Closed 2 years ago

Crash in [@ nsDocShell::IsSameDocumentNavigation]

Categories

(Thunderbird :: General, defect)

x86
All
defect

Tracking

(thunderbird_esr78+ fixed, thunderbird81 fixed)

RESOLVED FIXED
82 Branch
Tracking Status
thunderbird_esr78 + fixed
thunderbird81 --- fixed

People

(Reporter: wsmwk, Assigned: mkmelin)

Details

(Keywords: crash, regression, regressionwindow-wanted, Whiteboard: [regression:tb78])

Crash Data

Attachments

(1 file)

This bug is for crash report bp-ee43cf6d-b2f0-41eb-a501-9d9c50200718 78.0.1
bp-e0184cad-248d-4aec-9c03-2c4510200726 78.0

Top 10 frames of crashing thread:

0 xul.dll nsDocShell::IsSameDocumentNavigation docshell/base/nsDocShell.cpp:8250
1 xul.dll nsDocShell::InternalLoad docshell/base/nsDocShell.cpp:8588
2 xul.dll nsDocShell::LoadURI docshell/base/nsDocShell.cpp:807
3 xul.dll nsMailboxService::FetchMessage comm/mailnews/local/src/nsMailboxService.cpp:222
4 xul.dll nsMailboxService::DisplayMessage comm/mailnews/local/src/nsMailboxService.cpp:252
5 xul.dll nsMessenger::OpenURL comm/mailnews/base/src/nsMessenger.cpp:433
6 xul.dll nsMsgDBView::LoadMessageByViewIndex comm/mailnews/base/src/nsMsgDBView.cpp:1073
7 xul.dll nsMsgDBView::SelectionChangedXPCOM comm/mailnews/base/src/nsMsgDBView.cpp:1157
8 xul.dll NS_InvokeByIndex 
9 xul.dll static XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1140

https://hg.mozilla.org/releases/mozilla-esr78/file/tip/docshell/base/nsDocShell.cpp#l8250

nsresult rvURINew = aLoadState->URI()->GetRef(aState.mNewHash);

I guess aLoadState->URI() is null?

Worthy of a patch? Or are you asking Ben?

Flags: needinfo?(mkmelin+mozilla)

Looking at the stack this looks like our fault. The else clause above has cases where the url we use for loadinfo could be null.

https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/docshell/base/nsDocShellLoadState.cpp#146 asserts ... but that doesn't help us.

Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Flags: needinfo?(mkmelin+mozilla)
Attachment #9172966 - Flags: review?(benc)
Comment on attachment 9172966 [details] [diff] [review]
bug1655627_mailboxservice_crash.patch

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

Provisional r+ because this does look like it'd prevent the crashes we see at the moment.
But... if the aDisplayConsumer isn't a docshell, that null url will cause a crash (well... an assert) down in RunMailboxUrl() (https://searchfox.org/comm-central/search?q=nsMailboxService%3A%3ARunMailboxUrl) instead, where it calls 
nsMailboxProtocol::Initialize() (https://searchfox.org/comm-central/search?q=nsMailboxProtocol%3A%3AInitialize).

Not sure how much of an issue this is in practice though.
Attachment #9172966 - Flags: review?(benc) → review+

Right, that would only assert (in debug builds) which seems alright.

Target Milestone: --- → 82 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/021daf46287d
don't create and use a loadinfo with null url. r=benc

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED

Comment on attachment 9172966 [details] [diff] [review]
bug1655627_mailboxservice_crash.patch

[Approval Request Comment]
Crash fix

Attachment #9172966 - Flags: approval-comm-esr78?
Attachment #9172966 - Flags: approval-comm-beta?

Comment on attachment 9172966 [details] [diff] [review]
bug1655627_mailboxservice_crash.patch

[Triage Comment]
Approved for beta (by the time we release it will have been on nightly for several days)

Attachment #9172966 - Flags: approval-comm-beta? → approval-comm-beta+

Comment on attachment 9172966 [details] [diff] [review]
bug1655627_mailboxservice_crash.patch

[Triage Comment]
Approved for esr78. (although comment 8 has not yet come true)

Attachment #9172966 - Flags: approval-comm-esr78? → approval-comm-esr78+
Duplicate of this bug: 1658363
You need to log in before you can comment on or make changes to this bug.