1559523 - Crash in [@ mozilla::dom::Document::EndLoad] - Crash when using chat on Thunderbird trunk

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Jorg K (CEST = GMT+2)]

This bug is for crash report bp-aa20b0de-671c-4dcb-a441-2c88c0190614.

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::Document::EndLoad dom/base/Document.cpp:7093
1 xul.dll nsHtml5TreeOpExecutor::DidBuildModel parser/html/nsHtml5TreeOpExecutor.cpp:202
2 xul.dll nsHtml5TreeOpExecutor::FlushDocumentWrite parser/html/nsHtml5TreeOpExecutor.cpp:623
3 xul.dll nsHtml5Parser::ParseUntilBlocked parser/html/nsHtml5Parser.cpp:577
4 xul.dll nsHtml5Parser::Parse parser/html/nsHtml5Parser.cpp:213
5 xul.dll mozilla::dom::Document::Close dom/base/Document.cpp:8824
6 xul.dll static bool mozilla::dom::Document_Binding::close dom/bindings/DocumentBinding.cpp:3236
7 xul.dll mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3171
8 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:540
9 xul.dll static bool InternalCall js/src/vm/Interpreter.cpp:595

This happens when connecting to IRC in Thunderbird. In the debugger, I see this crash:

xul.dll!mozilla::dom::Document::EndLoad() Line 7093	C++
xul.dll!nsHtml5TreeOpExecutor::DidBuildModel(bool) Line 210	C++
xul.dll!nsHtml5TreeOpExecutor::FlushDocumentWrite() Line 625	C++
xul.dll!nsHtml5Parser::ParseUntilBlocked() Line 580	C++
xul.dll!nsHtml5Parser::Parse(const nsTSubstring<char16_t> & aSourceBuffer, void * aKey, bool) Line 213	C++
xul.dll!mozilla::dom::Document::Close(mozilla::ErrorResult & rv) Line 8824	C++
xul.dll!mozilla::dom::Document_Binding::close(JSContext * cx, JS::Handle<JSObject *> obj, mozilla::dom::Document * self, const JSJitMethodCallArgs & args) Line 3237	C++
xul.dll!mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy,mozilla::dom::binding_detail::ThrowExceptions>(JSContext * cx, unsigned int argc, JS::Value * vp) Line 3173	C++
 	xul.dll!CallJSNative(JSContext * cx, bool(*)(JSContext *, unsigned int, JS::Value *) native, const JS::CallArgs & args) Line 448	C++

Comment 1

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/mozilla-central/rev/72e19189bb99e159b2974cbcda69647211962df3 changed that file last. The crash started with today's Daily.

Comment 2

[Jorg K (CEST = GMT+2)]

Backing that out fixes the crash.

Comment 3

[Jorg K (CEST = GMT+2)]

Magnus, this seems related to the XUL to XHTML effort.

Comment 4

[Jorg K (CEST = GMT+2)]

Umm, I gave you the exact regression in comment #2.

Comment 6

[Magnus Melin [:mkmelin]]

Likely has something to do with the conversation-browser loading an internal html page which it then adds conversations to? https://searchfox.org/comm-central/rev/630f951ef8efd45af34ef07382851a4ab3184d6c/chat/content/conversation-browser.js#270

Seems to always crash the first time after starting, then not later (but then chat is not usable).

Comment 7

[Magnus Melin [:mkmelin]]

Crash should be here: https://searchfox.org/comm-central/rev/630f951ef8efd45af34ef07382851a4ab3184d6c/mozilla/dom/base/Document.cpp#7094

Comment 8

[Jorg K (CEST = GMT+2)]

That's what comment #1 says: 0 xul.dll mozilla::dom::Document::EndLoad dom/base/Document.cpp:7093

Comment 9

[Magnus Melin [:mkmelin]]

It's indeed related to the load of chrome://chat/content/conv.html - but no idea why it's a problem.
If I put back the check for IsXULDocument() and add an else case (which would be the crash), that else case is where where load conv.html

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Magnus Melin [:mkmelin] from comment #7)

Crash should be here:
https://searchfox.org/comm-central/rev/
630f951ef8efd45af34ef07382851a4ab3184d6c/mozilla/dom/base/Document.cpp#7094

That link gives just "File not found"

Comment 11

[Jorg K (CEST = GMT+2)]

Yes, because searchfox ... comm-central ... mozilla links don't work :-( - Magnus, DO NOT post such links.

Here's the right one at Document.cpp#7094. Sadly Searchfox' permalinks don't use HG versions (and MXR either), so it's a pain to find now:

https://hg.mozilla.org/mozilla-central/file/72e19189bb99e159b2974cbcda69647211962df3/dom/base/Document.cpp#l7093

NS_DOCUMENT_NOTIFY_OBSERVERS(EndLoad, (this)); crashes ... and it looks like the line has moved now.

Comment 12

[Jorg K (CEST = GMT+2)]

Moved to
https://searchfox.org/mozilla-central/rev/da14c413ef663eb1ba246799e94a240f81c42488/dom/base/Document.cpp#7189
and still crashing, see for example https://crash-stats.mozilla.org/report/index/c8163bd7-f678-4ea4-bace-94cbe0190621:
0 XUL mozilla::dom::Document::EndLoad() dom/base/Document.cpp:7189

Comment 13

[Kai Engert [:KaiE:]]

I also run into the crash. I'll start a debug build, maybe looking at the state of the variables gives us a clue?

Comment 14

[Brendan Dahl [:bdahl]]

If you have reliable steps to reproduce in a debug build, I'd be happy to take a look.

Comment 15

[Kai Engert [:KaiE:]]

Can you build a Thunderbird debug build?

All I have a do is:

• start Thunderbird
• open the chat tab
• I see an online buddy, and I click that buddy

That crashes, probably trying to prepare the window that's used for a conversation with the buddy.

Comment 16

[Jorg K (CEST = GMT+2)]

I crash when opening TB chat since I auto-connect to #maildev on Mozilla's IRC and that opens a conversation. Please note comment #6 and comment #9. Crash location in comment #12.

Comment 17

[Kai Engert [:KaiE:]]

Attachment: helpdebug.patch

The crash is inside a macro. The macro defines a loop, which makes debugging difficult. This patch expands the macro. I've also introduced a temporary variable, to save the value obtained from the call to GetNext()

Comment 18

[Kai Engert [:KaiE:]]

Thread 1 "thunderbird" received signal SIGSEGV, Segmentation fault.
0x00007fffe8c28c71 in mozilla::RefPtrTraits<nsIDocumentObserver>::AddRef (aPtr=0x7fffcd779ea0) at /home/user/moz/commcent/obj-thunder-debug/dist/include/mozilla/RefPtr.h:45
45	  static void AddRef(U* aPtr) { aPtr->AddRef(); }
(gdb) bt 15
#0  0x00007fffe8c28c71 in mozilla::RefPtrTraits<nsIDocumentObserver>::AddRef(nsIDocumentObserver*) (aPtr=0x7fffcd779ea0) at /home/user/moz/commcent/obj-thunder-debug/dist/include/mozilla/RefPtr.h:45
#1  0x00007fffe8c28c05 in RefPtr<nsIDocumentObserver>::ConstRemovingRefPtrTraits<nsIDocumentObserver>::AddRef(nsIDocumentObserver*) (aPtr=0x7fffcd779ea0)
    at /home/user/moz/commcent/obj-thunder-debug/dist/include/mozilla/RefPtr.h:362
#2  0x00007fffe8c28bdc in RefPtr<nsIDocumentObserver>::assign_with_AddRef(nsIDocumentObserver*) (this=0x7fffffff6740, aRawPtr=0x7fffcd779ea0)
    at /home/user/moz/commcent/obj-thunder-debug/dist/include/mozilla/RefPtr.h:56
#3  0x00007fffe8b8c1df in RefPtr<nsIDocumentObserver>::operator=(nsIDocumentObserver*) (this=0x7fffffff6740, aRhs=0x7fffcd779ea0) at /home/user/moz/commcent/obj-thunder-debug/dist/include/mozilla/RefPtr.h:178
#4  0x00007fffe8b8db71 in mozilla::dom::Document::EndLoad() (this=0x7fffcd78d000) at /home/user/moz/commcent/mozilla/dom/base/Document.cpp:7264
#5  0x00007fffe811a1fa in nsHtml5TreeOpExecutor::DidBuildModel(bool) (this=0x7fffd1026400, aTerminated=false) at /home/user/moz/commcent/mozilla/parser/html/nsHtml5TreeOpExecutor.cpp:202
#6  0x00007fffe811e66e in nsHtml5TreeOpExecutor::FlushDocumentWrite() (this=0x7fffd1026400) at /home/user/moz/commcent/mozilla/parser/html/nsHtml5TreeOpExecutor.cpp:623
#7  0x00007fffe80cf28a in nsHtml5Parser::ParseUntilBlocked() (this=0x7fffcde33f90) at /home/user/moz/commcent/mozilla/parser/html/nsHtml5Parser.cpp:577
#8  0x00007fffe80cdd46 in nsHtml5Parser::Parse(nsTSubstring<char16_t> const&, void*, bool) (this=0x7fffcde33f90, aSourceBuffer=..., aKey=0x0, aLastCall=true)
    at /home/user/moz/commcent/mozilla/parser/html/nsHtml5Parser.cpp:213
#9  0x00007fffe8b9916b in mozilla::dom::Document::Close(mozilla::ErrorResult&) (this=0x7fffcd78d000, rv=...) at /home/user/moz/commcent/mozilla/dom/base/Document.cpp:9006
#10 0x00007fffe9c5118d in mozilla::dom::Document_Binding::close(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Document*, JSJitMethodCallArgs const&)
    (cx=0x7fffe422d000, obj=..., self=0x7fffcd78d000, args=...) at DocumentBinding.cpp:3236
#11 0x00007fffe9fc1837 in mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)
    (cx=0x7fffe422d000, argc=0, vp=0x7fffcddc6158) at /home/user/moz/commcent/mozilla/dom/bindings/BindingUtils.cpp:3171
#12 0x00007fffee153b7f in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)
    (cx=0x7fffe422d000, native=0x7fffe9fc1560 <mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/user/moz/commcent/mozilla/js/src/vm/Interpreter.cpp:448
#13 0x00007fffee15347a in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=0x7fffe422d000, args=..., construct=js::NO_CONSTRUCT)
    at /home/user/moz/commcent/mozilla/js/src/vm/Interpreter.cpp:540
#14 0x00007fffee1546c5 in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7fffe422d000, args=...) at /home/user/moz/commcent/mozilla/js/src/vm/Interpreter.cpp:595
(More stack frames follow...)
(gdb) up 4
#4  0x00007fffe8b8db71 in mozilla::dom::Document::EndLoad (this=0x7fffcd78d000) at /home/user/moz/commcent/mozilla/dom/base/Document.cpp:7264
7264	        obs_ = next;
(gdb) print next
$1 = (nsIDocumentObserver *) 0x7fffcd779ea0
(gdb) print *next
$2 = {<nsIMutationObserver> = {<nsISupports> = {_vptr$nsISupports = 0xe5e5e5e5e5e5e5e5}, <No data fields>}, <No data fields>}

Comment 19

[Kai Engert [:KaiE:]]

(gdb) print iter_
$3 = {<nsAutoTObserverArray<nsIDocumentObserver*, 0>::Iterator> = {<nsTObserverArray_base::Iterator_base> = {mPosition = 1, mNext = 0x0}, mArray = @0x7fffcd78d558}, <No data fields>}

Does the iterator return an object that has already been deleted?

Comment 20

[Kai Engert [:KaiE:]]

Attachment: 1559523-v1.patch

I don't know how this code works, but limiting XulPersist to XulDocuments seems reasonable.
This patch fixes the crash for me.

Comment 21

[Richard Marti (:Paenglab)]

It crashes too when you switch in Prefs/Chat to the "Message Styles" tab.

Comment 22

[Takanori MATSUURA]

This bug should be fixed immediately because we face to the crash loop once we get this crash.

• After restarting Tb, the last tab is activated. This means the activated tab is Prefs/Chat and Tb crash again.
  • To escape crash loop, we need to close Prefs tab immediately after restarting Tb.
• Tb crash again if we reopen the Prefs tab because Prefs tab opens the last one (Prefs/Chat).
  • To escape crash loop, we need to select other category from the left of Prefs immediately after opening Prefs tab.

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9074886 [details] [diff] [review]
1559523-v1.patch

This is basically backing out the main piece of bug 1552049.

Comment 24

[Kai Engert [:KaiE:]]

Attachment: 1559523-v2.patch

Olli suggested this change, which fixes the crash for me.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2514efbfbb78
Fix a crash in Thunderbird, [@ mozilla::dom::Document::EndLoad]. r=kaie

Comment 26

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/2514efbfbb78