stylo: panicked at '<div> has still dirty bit true or animation-only dirty bit false'

RESOLVED FIXED in Firefox 57

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
9 months ago
8 months ago

People

(Reporter: truber, Assigned: emilio)

Tracking

(Blocks: 5 bugs, {assertion, testcase})

Trunk
mozilla57
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 wontfix, firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

9 months ago
Created attachment 8896455 [details]
testcase.html

The attached testcase causes a panic in m-c rev 65826179c86e with stylo enabled by pref and e10s disabled.

The panic seems intermittent but usually occurs within 1-10 cycles of the testcase (testcase calls `location.reload()`).

The domFuzzLite extension [1] is required to trigger garbage & cycle-collection.

1. https://github.com/MozillaSecurity/domfuzz/tree/master/dom/extension

thread '<unnamed>' panicked at '<div> (0x7fe6a78274c0) has still dirty bit true or animation-only dirty bit false', /home/worker/workspace/build/src/servo/ports/geckolib/glue.rs:3240
stack backtrace:
   0:     0x7fe6c8908813 - std::sys::imp::backtrace::tracing::imp::unwind_backtrace::hcab99e0793da62c7
   1:     0x7fe6c8903b36 - std::sys_common::backtrace::_print::hbfe5b0c7e79c0711
   2:     0x7fe6c8915eaa - std::panicking::default_hook::{{closure}}::h9ba2c6973907a2be
   3:     0x7fe6c8915aab - std::panicking::default_hook::he4d55e2dd21c3cca
   4:     0x7fe6c89162fa - std::panicking::rust_panic_with_hook::ha138c05cd33ad44d
   5:     0x7fe6c8916194 - std::panicking::begin_panic::hcdbfa35c94142fa2
   6:     0x7fe6c89160c9 - std::panicking::begin_panic_fmt::hc09fe500d9b7be81
   7:     0x7fe6c8306011 - geckoservo::glue::Servo_AssertTreeIsClean::assert_subtree_is_clean::h0032d76c72267e31
   8:     0x7fe6c8305f18 - geckoservo::glue::Servo_AssertTreeIsClean::assert_subtree_is_clean::h0032d76c72267e31
   9:     0x7fe6c8305f18 - geckoservo::glue::Servo_AssertTreeIsClean::assert_subtree_is_clean::h0032d76c72267e31
  10:     0x7fe6c6735ff0 - mozilla::ServoStyleSet::AssertTreeIsClean()
  11:     0x7fe6c681a3a5 - mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags)
  12:     0x7fe6c681d9e1 - mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush)
  13:     0x7fe6c67eb4a9 - nsRefreshDriver::Tick(long, mozilla::TimeStamp)
  14:     0x7fe6c67ec591 - mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&)
  15:     0x7fe6c67ec67d - mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp)
  16:     0x7fe6c67ec840 - mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp)
  17:     0x7fe6c67ecb90 - mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run()
  18:     0x7fe6c4af58fd - nsThread::ProcessNextEvent(bool, bool*) [clone .part.239]
  19:     0x7fe6c4af14f6 - NS_ProcessNextEvent(nsIThread*, bool)
  20:     0x7fe6c4ea60e6 - mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)
  21:     0x7fe6c4e7c01a - MessageLoop::RunInternal()
  22:     0x7fe6c4e7c046 - MessageLoop::Run()
  23:     0x7fe6c65e61ce - nsBaseAppShell::Run()
  24:     0x7fe6c7514f82 - nsAppStartup::Run()
  25:     0x7fe6c75982da - XREMain::XRE_mainRun()
  26:     0x7fe6c7598ad4 - XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&)
  27:     0x7fe6c7598dbf - XRE_main(int, char**, mozilla::BootstrapConfig const&)
  28:           0x4064b5 - do_main(int, char**, char**)
  29:           0x405c55 - main
  30:     0x7fe6d5fa34c9 - __libc_start_main
  31:           0x405ec8 - <unknown>
Redirecting call to abort() to mozalloc_abort
Great work Jesse!

(In reply to Jesse Schwartzentruber (:truber) from comment #0)

> thread '<unnamed>' panicked at '<div> (0x7fe6a78274c0) has still dirty bit
> true or animation-only dirty bit false',

Interesting, I thought animation dirty bit was true.

I will look this next week, but if someone wants to look it, please feel free take over.
Assignee: nobody → hikezoe
I could reproduce the crash on 65826179c86e but could not reproduce on today's mozilla-central (3bfcbdf5c6c3).
Jesse, could you please double-check that the crash does not happen on the latest mozilla-central?

Also, unfortunately I can't write automation test for this crash, I did replace both of fuzzPriv.forceGC() and fuzzPriv.CC() with DOMWindowUtils.forceGC() and DOMWindowUtils.cycleCollect(), but no luck.
Flags: needinfo?(jschwartzentruber)
(Reporter)

Comment 3

8 months ago
Hi Hiro,

No, I am not able to reproduce this anymore either. I have two cases I have seen:

thread '<unnamed>' panicked at '<tbody> (0x60d0005545b0) has still dirty bit true or animation-only dirty bit false', /home/worker/workspace/build/src/servo/ports/geckolib/glue.rs:3241

  Was happening regularly from 20170728-16ffc1d05422 (central), but seems to have stopped after 20170811-2b8fe8c61b34 (inbound).

thread '<unnamed>' panicked at '<body> (0x7fdf746d5fb0) has still dirty bit true or animation-only dirty bit true', /home/worker/workspace/build/src/servo/ports/geckolib/glue.rs:3066

  Have only seen this twice, both on inbound: 20170723-7f8e505bfa44 and 20170722-f5acec377801.

Sorry, I must have copied the wrong panic when filing this.
Flags: needinfo?(jschwartzentruber)
No worries! Thanks for the check! I am going to close this bug as WORKSFORME.

(In reply to Jesse Schwartzentruber (:truber) from comment #3)
> thread '<unnamed>' panicked at '<body> (0x7fdf746d5fb0) has still dirty bit
> true or animation-only dirty bit true',
> /home/worker/workspace/build/src/servo/ports/geckolib/glue.rs:3066

This case is really interesting. Both of dirty bits remain.  I am really relived that this panic has been already solved.
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 5

8 months ago
Created attachment 8900783 [details]
testcase-new.html

Here is a new testcase that reproduces on m-c rev 20170824-892c8916ba32 with stylo enabled by pref and e10s can be enabled.
Attachment #8896455 - Attachment is obsolete: true
(Reporter)

Updated

8 months ago
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(Reporter)

Updated

8 months ago
Blocks: 1340565
I got an RR recording, and a patch that I'm testing through try right now.
Assignee: hikezoe → emilio+bugs
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 9

8 months ago
mozreview-review
Comment on attachment 8901204 [details]
Bug 1389645: Don't incorrectly set lazy frame construction bits in ContentAppended and ContentRangeInserted.

https://reviewboard.mozilla.org/r/172672/#review178190

::: layout/base/nsCSSFrameConstructor.cpp:7686
(Diff revision 1)
>        // The Servo-backed style system handles this case like the lazy frame
>        // construction case.

Expand this comment to include discussion of the failure mode for this bug?
Attachment #8901204 - Flags: review?(bobbyholley) → review+

Comment 10

8 months ago
mozreview-review
Comment on attachment 8901203 [details]
stylo: Make Servo_TraverseSubtree and Servo_AssertTreeIsClean output more useful info.

https://reviewboard.mozilla.org/r/172670/#review178192

r=me with that fixed. Thanks!

Don't forget to land the crashtest.

::: servo/ports/geckolib/glue.rs:275
(Diff revision 1)
>      debug_assert!(!snapshots.is_null());
>  
>      let element = GeckoElement(root);
>  
>      debug!("Servo_TraverseSubtree (flags={:?})", traversal_flags);
> +    debug!("{:?}", ShowSubtreeData(element.as_node()));

Won't this make us dump the subtree twice? Seems like we want to remove the dump from traverse_subtree.
Attachment #8901203 - Flags: review?(bobbyholley) → review+

Comment 11

8 months ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3311859df588
Don't incorrectly set lazy frame construction bits in ContentAppended and ContentRangeInserted. r=bholley
https://hg.mozilla.org/integration/autoland/rev/2884dad61e5e
Crashtest. r=me
status-firefox55: --- → unaffected
status-firefox56: --- → wontfix
status-firefox-esr52: --- → unaffected
Priority: -- → P2

Comment 12

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3311859df588
https://hg.mozilla.org/mozilla-central/rev/2884dad61e5e
Status: REOPENED → RESOLVED
Last Resolved: 8 months ago8 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(Reporter)

Comment 13

8 months ago
Created attachment 8903281 [details]
testcase-3.html

Another testcase which repros on m-c rev 20170831-fb22415719a9
Flags: needinfo?(emilio)
(Reporter)

Updated

8 months ago
Blocks: 331889
Nice! I confirmed that bug 1394935 fixed the test case in comment 13. We can use the test case for bug 1394935. Thank you Jesse!
Flags: needinfo?(emilio)
You need to log in before you can comment on or make changes to this bug.