1414913 - Assertion failure: !mMutationGuard.Mutated(0), at /builds/worker/workspace/build/src/dom/base/ChildIterator.h:210

Status: RESOLVED DUPLICATE of bug 1414762

(Core :: Graphics: ImageLib, defect)

Description

[Jason Kratzer [:jkratzer]]

Attachment: trigger.html

Testcase found while fuzzing mozilla-central rev dc45ee24c55d.

==12047==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f8bd6d91578 bp 0x7ffdab43ddf0 sp 0x7ffdab43dde0 T0)
==12047==The signal is caused by a WRITE memory access.
==12047==Hint: address points to the zero page.
    #0 0x7f8bd6d91577 in mozilla::dom::AllChildrenIterator::~AllChildrenIterator() /builds/worker/workspace/build/src/dom/base/ChildIterator.h:210:28
    #1 0x7f8bda5548c6 in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /builds/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:963:3
    #2 0x7f8bda556efc in mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /builds/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:1142:28
    #3 0x7f8bda524c7b in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/workspace/build/src/layout/base/PresShell.cpp:4196:41
    #4 0x7f8bda524541 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::FlushType) /builds/worker/workspace/build/src/layout/base/PresShell.cpp:4068:3
    #5 0x7f8bd6e0addb in mozilla::dom::Selection::ScrollIntoView(short, nsIPresShell::ScrollAxis, nsIPresShell::ScrollAxis, int) /builds/worker/workspace/build/src/dom/base/Selection.cpp:3687:16
    #6 0x7f8bd6e0f969 in mozilla::dom::Selection::ScrollSelectionIntoViewEvent::Run() /builds/worker/workspace/build/src/dom/base/Selection.cpp:3593:15
    #7 0x7f8bda4b84d6 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:1830:22
    #8 0x7f8bda4c1dae in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:306:7
    #9 0x7f8bda4c1b84 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:327:5
    #10 0x7f8bda4c5065 in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:769:5
    #11 0x7f8bda4c4106 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:682:35
    #12 0x7f8bda4c0287 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() /builds/worker/workspace/build/src/layout/base/nsRefreshDriver.cpp:528:20
    #13 0x7f8bd4808cff in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1037:14
    #14 0x7f8bd4829910 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:513:10
    #15 0x7f8bd53c6025 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21
    #16 0x7f8bd5318177 in MessageLoop::RunInternal() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #17 0x7f8bd5318009 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299:3
    #18 0x7f8bd9faae1a in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:158:27
    #19 0x7f8bdd1cefe1 in nsAppStartup::Run() /builds/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:288:30
    #20 0x7f8bdd343b68 in XREMain::XRE_mainRun() /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4675:22
    #21 0x7f8bdd34578a in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4837:8
    #22 0x7f8bdd3466b9 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4932:21
    #23 0x4ed558 in do_main(int, char**, char**) /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:231:22
    #24 0x4ece7b in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:304:16
    #25 0x7f8bf3aad82f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291

Comment 1

[Andrew Overholt [:overholt]]

Emilio knows the Servo side of this so may have an idea what's going on here.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Stack of the mutation.

Ugghh, this is kinda bad, and it's really happening somewhere where it shouldn't.

To be clear, this is not a Servo bug, Servo just happens to hold an iterator that asserts it, but there shouldn't be attribute change notifications, much less JS code running, from style updates.

In this case the code is under our control, and it's harmless. This is the attribute mutation:

  http://searchfox.org/mozilla-central/rev/7e090b227f7a0ec44d4ded604823d48823158c51/browser/base/content/tabbrowser.xml#669

I think it's not sane nor expected for this code to be running during layout...

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Also, this seems to only happen without e10s, but I could believe we have code on e10s doing similar stuff, and maybe that can even be accessible from content (that looks like the onreadystatechange event...)

Boris, I'm not familiar with nsDocLoader stuff, so I don't know what should be done to fix this... Should the request be cancelled async? Or the onreadystatechange event be dispatched async?

Comment 4

[Boris Zbarsky [:bzbarsky]]

imgRequestProxy::CancelAndForgetObserver needs to guarantee that it does not sync-remove the request from the loadgroup.  See bug 488850 comment 23, which more or less explains why this part HAS TO BE ASYNC.  Back when we fixed bug 488850 we ensured that mIsInLoadGroup was false across the RemoveProxy() call, precisely because of this problem (see the "oldIsInLoadGroup" bits).  Unfortunately it looks like the documentation there wasn't nearly clear enough.

So what happened is that the fixes in bug 1404422 effectively lost that part of the fix for bug 488850.  They kept the async removal via the actual RemoveFromLoadGroup() call, by setting mForceDispatchLoadGroup, but the RemoveProxy() is now outside that, and that's where the sync removal happens:

#35 0x00007f6627db2e6e in imgRequestProxy::RemoveFromLoadGroup() (this=0x7f660e765580) at /home/emilio/projects/moz/gecko/image/imgRequestProxy.cpp:410
#36 0x00007f6627db59a6 in imgRequestProxy::OnLoadComplete(bool) (this=0x7f660e765580, aLastPart=true) at /home/emilio/projects/moz/gecko/image/imgRequestProxy.cpp:1087
#37 0x00007f6627d8820f in mozilla::image::ProgressTracker::EmulateRequestFinished(mozilla::image::IProgressObserver*) (this=0x7f660ea244c0, aObserver=0x7f660e765588)
    at /home/emilio/projects/moz/gecko/image/ProgressTracker.cpp:422
#38 0x00007f6627d8505c in mozilla::image::ProgressTracker::RemoveObserver(mozilla::image::IProgressObserver*) (this=0x7f660ea244c0, aObserver=0x7f660e765588)
    at /home/emilio/projects/moz/gecko/image/ProgressTracker.cpp:506
#39 0x00007f6627dae3dc in imgRequest::RemoveProxy(imgRequestProxy*, nsresult) (this=0x7f65fe10c760, proxy=0x7f660e765580, aStatus=nsresult::NS_ERROR_FAILURE)
    at /home/emilio/projects/moz/gecko/image/imgRequest.cpp:253
#40 0x00007f6627db3ba3 in imgRequestProxy::CancelAndForgetObserver(nsresult) (this=0x7f660e765580, aStatus=nsresult::NS_ERROR_FAILURE)

I _think_ simply moving the 

  mForceDispatchLoadGroup = true;

bit to before the RemoveProxy() call, and restoring the comments that explain why the ordering needs to be that way, might be enough here, but going to defer to Andrew on that.

Comment 5

[Boris Zbarsky [:bzbarsky]]

The good news is that this is nightly-only so far, by the way.

Comment 6

[Timothy Nikkel (:tnikkel)]

Bug 1414762 should fix this.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

Yes, you are correct. I was aware imgRequest::RemoveProxy explicitly removed the request from the load group (and I moved that code elsewhere) but I had missed the ways it could indirectly do so. It is fixed as of build 20171107220115.

Comment 8

[Al Billings [:abillings - ex-MoCo]]

Jason, can you see if this is fixed now?

Comment 9

[Jason Kratzer [:jkratzer]]

(In reply to Al Billings [:abillings] from comment #8)
> Jason, can you see if this is fixed now?

I just tested against mozilla-central rev d16b52f5d195 (20171109) and can confirm this is fixed.

Comment 10

[Gerry Chang [:gchang]]

Per comment #9, it's fixed. Track 58-.