Closed Bug 1588259 Opened 5 years ago Closed 5 years ago

Assertion failure: !chan (Why is there a document channel?), at src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:996

Categories

(Core :: DOM: Navigation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla73
Fission Milestone M5
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- wontfix
firefox72 --- wontfix
firefox73 --- fixed

People

(Reporter: tsmith, Assigned: kmag)

References

(Blocks 1 open bug, Regression)

Details

(4 keywords, Whiteboard: [retriggered], [stockwell disable-recommended])

Attachments

(4 files)

Attached file testcase.html (deleted) —

Reduced with m-c:
BuildID=20191011093010
SourceStamp=a7ca5ad33f3df4477ddb16560386b1b5aa641c3b

Testcase requires disable_open_during_load=false to reproduce the issue.

Assertion failure: !chan (Why is there a document channel?), at src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:996

0|0|libxul.so|nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy*, char const*, char const*, char const*, bool, bool, bool, nsIArray*, bool, bool, bool, nsDocShellLoadState*, mozilla::dom::BrowsingContext**)|hg:hg.mozilla.org/mozilla-central:toolkit/components/windowwatcher/nsWindowWatcher.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|683|0x3f
0|1|libxul.so|nsWindowWatcher::OpenWindow2(mozIDOMWindowProxy*, char const*, char const*, char const*, bool, bool, bool, nsISupports*, bool, bool, bool, nsDocShellLoadState*, mozilla::dom::BrowsingContext**)|hg:hg.mozilla.org/mozilla-central:toolkit/components/windowwatcher/nsWindowWatcher.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|379|0xa
0|2|libxul.so|nsGlobalWindowOuter::OpenInternal(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, bool, bool, bool, bool, bool, nsIArray*, nsISupports*, nsDocShellLoadState*, bool, mozilla::dom::BrowsingContext**)|hg:hg.mozilla.org/mozilla-central:dom/base/nsGlobalWindowOuter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|7246|0x5f
0|3|libxul.so|nsGlobalWindowOuter::OpenJS(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::dom::BrowsingContext**)|hg:hg.mozilla.org/mozilla-central:dom/base/nsGlobalWindowOuter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|5765|0x18
0|4|libxul.so|nsGlobalWindowOuter::OpenOuter(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&)|hg:hg.mozilla.org/mozilla-central:dom/base/nsGlobalWindowOuter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|5729|0x1f
0|5|libxul.so|nsGlobalWindowInner::Open(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&)|hg:hg.mozilla.org/mozilla-central:dom/base/nsGlobalWindowInner.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|3772|0x29
0|6|libxul.so|mozilla::dom::Window_Binding::open|s3:gecko-generated-sources:69b81aac238fb2c3636a98f044e80fbb6d1d2f3d96731bb4792037410341ae71dfcc1fee0a36805040045dcb619619a99678d3ce937864c138673d150964450f/dom/bindings/WindowBinding.cpp:|2867|0x2d
0|7|libxul.so|bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::MaybeCrossOriginObjectThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)|hg:hg.mozilla.org/mozilla-central:dom/bindings/BindingUtils.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|3250|0x24
0|8|libxul.so|CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|457|0x15
0|9|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|549|0x15
0|10|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|618|0x10
0|11|libxul.so|Interpret|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|622|0x15
0|12|libxul.so|js::RunScript(JSContext*, js::RunState&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|424|0xb
0|13|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|590|0x13
0|14|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|618|0x10
0|15|libxul.so|js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|635|0x8
0|16|libxul.so|JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/jsapi.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|2722|0x1f
0|17|libxul.so|mozilla::dom::MessageListener::ReceiveMessage(JSContext*, JS::Handle<JS::Value>, mozilla::dom::ReceiveMessageArgument const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&)|s3:gecko-generated-sources:acb3317ac9d61072476fc2d6ab9176bbb27feb31767ae7cd988b70851fa5be542fe03c469f03a486a2810b4af28eeab4b9162824485f333d59e63670d387c6d9/dom/bindings/MessageManagerBinding.cpp:|7133|0x5
0|18|libxul.so|nsFrameMessageManager::ReceiveMessage(nsISupports*, nsFrameLoader*, bool, nsTSubstring<char16_t> const&, bool, mozilla::dom::ipc::StructuredCloneData*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::dom::ipc::StructuredCloneData>*, mozilla::ErrorResult&)|s3:gecko-generated-sources:1da160f012ddb89bbe79c69f36ef267f43ae64110996cf1d23d0a8d3ee8d408cc7ec7c4b7706e07605c4f95cd1f997c55066f7671308d4e706b4342fb7345bca/dist/include/mozilla/dom/MessageManagerBinding.h:|776|0x23
0|19|libxul.so|nsFrameMessageManager::ReceiveMessage(nsISupports*, nsFrameLoader*, nsTSubstring<char16_t> const&, bool, mozilla::dom::ipc::StructuredCloneData*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::dom::ipc::StructuredCloneData>*, mozilla::ErrorResult&)|hg:hg.mozilla.org/mozilla-central:dom/base/nsFrameMessageManager.h:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|246|0x20
0|20|libxul.so|mozilla::dom::BrowserChild::RecvAsyncMessage(nsTString<char16_t> const&, nsTArray<mozilla::jsipc::CpowEntry>&&, nsIPrincipal*, mozilla::dom::ClonedMessageData const&)|hg:hg.mozilla.org/mozilla-central:dom/ipc/BrowserChild.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|2239|0x21
0|21|libxul.so|mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&)|s3:gecko-generated-sources:84c23ddf36d28d54592fdfd39a82edc0df0d230ab7a09a827af294045764b77b75846c8e6784029203b6b7ef8fc8a28ad34e8c109f541581145bf5e0ebc07b32/ipc/ipdl/PBrowserChild.cpp:|4000|0x5
0|22|libxul.so|mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&)|s3:gecko-generated-sources:fd8cde07bc12da00eb110c7ee29edc1f156c76a3c38880b5eb998e6f5a508d631a6ba97e85c721400241c86c5644225bfec5fe45de8b84ef4fe326e48c5521d0/ipc/ipdl/PContentChild.cpp:|7834|0x15
0|23|libxul.so|mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|2185|0x6
0|24|libxul.so|mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|2109|0xb
0|25|libxul.so|mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|1954|0xb
0|26|libxul.so|mozilla::ipc::MessageChannel::MessageTask::Run()|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|1985|0xc
0|27|libxul.so|mozilla::SchedulerGroup::Runnable::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/SchedulerGroup.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|295|0x15
0|28|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|1225|0x15
0|29|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|486|0x11
0|30|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|110|0xd
0|31|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|315|0x17
0|32|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|290|0x8
0|33|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|137|0xd
0|34|libxul.so|XRE_RunAppShell()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|934|0x11
0|35|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|238|0x5
0|36|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|315|0x17
0|37|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|290|0x8
0|38|libxul.so|XRE_InitChildProcess(int, char**, XREChildData const*)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|769|0xc
0|39|firefox-bin|content_process_main(mozilla::Bootstrap*, int, char**)|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|56|0x14
0|40|firefox-bin|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:a7ca5ad33f3df4477ddb16560386b1b5aa641c3b|272|0x12
0|41|libc-2.23.so|__libc_start_main|||0xf0
0|42|firefox-bin|_start|||0x29
Flags: in-testsuite?

The priority flag is not set for this bug.
:enndeakin, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(enndeakin)

Can reproduce most of the time, but has something to do with document loading and not window handling.

Component: Window Management → Document Navigation
Flags: needinfo?(enndeakin)
Flags: needinfo?(matt.woodrow)
Priority: -- → P3
Fission Milestone: --- → M5
Attached file stack (deleted) —

This is the stack that creates the load during window opening.

We're spinning a nested event loop, and recursing into clicking the form elements submit button.

I'm honestly surprised that more things don't entirely break.

Kris, is it ever going to be feasible to not have a nested event loop?

Failing that, I think we just need to downgrade this to a non-fatal assertion, or restore the code that manually stopped any in-progress loads. It's hard to guarantee the state if we can run arbitrary events.

Flags: needinfo?(matt.woodrow) → needinfo?(kmaglione+bmo)
Attachment #9115671 - Attachment mime type: application/octet-stream → text/plain
Flags: needinfo?(kmaglione+bmo)

(In reply to Matt Woodrow (:mattwoodrow) from comment #3)

Kris, is it ever going to be feasible to not have a nested event loop?

Eventually, yes, though how far in the future that will be, I don't know.

Failing that, I think we just need to downgrade this to a non-fatal assertion, or restore the code that manually stopped any in-progress loads. It's hard to guarantee the state if we can run arbitrary events.

I don't think that's the answer. I think there are at least two things we need to do here:

  1. Suppress event handling for all documents in the browsing context group while we're spinning the event loop, which will prevent the event handlers that cause the re-entrancy from firing in the first place.

  2. Abort the load attempt as early as possible when we detect this kind of re-entrancy. I'm not sure how aggressive we need to be about that, but we can probably just deny any targeted load attempt for a BrowsingContextGroup when we're waiting for a window.open() to complete for any other BrowsingContext in that group.

Depends on: 1603841

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression

There are 92 failures in the last 6 days.
All of them are on Linux64 - debug

Whiteboard: [retriggered] → [retriggered], [stockwell needswork]

I haven't been able to reproduce this so far, are there some special steps to take?

Assignee: nobody → kmaglione+bmo

(In reply to Dave Townsend [:mossop] (he/him) from comment #11)

I haven't been able to reproduce this so far, are there some special steps to take?

I was able to reproduce the issue using the information from the initial submission.

A Pernosco session is available here: https://pernos.co/debug/bVyvx3KJNllEhcokvK5prg/index.html

This doesn't solve all problems with potential reentrancy during window.open
nested event loops, but it does improve the situation somewhat. Since any
window in the same BrowsingContextGroup can target any window in the same
group, we need to suspend all windows in the group, not just the root of the
new window's parent. We also need to make sure we suspend all in-process
windows, even if we have out-of-process frames somewhere in the parent chain.

This patch takes care of suspending timeouts and input event handling in all
of these cases. It doesn't block all potential paths for running code in the
suspended windows, though, so the next patch explicitly prevents the
problematic reentrancy.

When we open a new window from a content process, we create a nested event
loop to wait for it to be initialized by the parent. The problem with this is
that the OpenWindow code which calls the window provider expects the window to
be in-process and uninitialized, so that it can load its own initial URI into
it, and correctly fulfil the spec-codified contract of window.open(). If
another caller initiates a load in the new window during the nested event
loop, those invariants are broken, and any manner of undefined behavior can
occur.

This patch adds a new flag to the BrowsingContext, marking it as uninitialized
until the end of the nested event loop, and blocking any attempts to load a
new URI into it in the meantime.

Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/34716f07e8cb
Part 1 - Suspend windows when spinning event loop for window.open. r=smaug
https://hg.mozilla.org/integration/autoland/rev/62fb499df45a
Part 2 - Prevent URI loads in partially-initialized BrowsingContexts. r=smaug
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/489a4cad2388
Part 1 - Suspend windows when spinning event loop for window.open. r=smaug
https://hg.mozilla.org/integration/autoland/rev/6a2972c587b6
Part 2 - Prevent URI loads in partially-initialized BrowsingContexts. r=smaug
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Regressions: 1605321
Regressions: 1605433
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(dtownsend)
Flags: in-testsuite? → in-testsuite+
Regressions: 1611986
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: