Closed Bug 1379585 Opened 2 years ago Closed 2 years ago

stylo: Crash in geckoservo::glue::Servo_StyleSet_FlushStyleSheets

Categories

(Core :: CSS Parsing and Computation, defect, P1, critical)

56 Branch
Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed

People

(Reporter: calixte, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-fb204118-6413-45aa-a437-f70e10170708.
=============================================================

There are 3 crashes in nightly 56.
Crash Signature: [@ geckoservo::glue::Servo_StyleSet_FlushStyleSheets] → [@ geckoservo::glue::Servo_StyleSet_FlushStyleSheets] [@ alloc::oom::default_oom_handler | geckoservo::glue::Servo_StyleSet_FlushStyleSheets]
Priority: -- → P1
I see from the report that we're already inside a call to Servo_StyleSet_FlushStyleSheets (frame 42) and a sequence of calls is causing the function to be called again, effectively recursively. I assume that causes global_style_data.shared_lock.read() to panic. This could probably be fixed by having Servo_StyleSet_FlushStyleSheets detect the recursion and early-exit on the second call, but if there's some other problem with the call stack chain, we should fix that instead.
Crash Signature: [@ geckoservo::glue::Servo_StyleSet_FlushStyleSheets] [@ alloc::oom::default_oom_handler | geckoservo::glue::Servo_StyleSet_FlushStyleSheets] → [@ alloc::oom::default_oom_handler | geckoservo::glue::Servo_StyleSet_FlushStyleSheets] [@ geckoservo::glue::Servo_StyleSet_FlushStyleSheets] [@ mozalloc_abort | abort | geckoservo::glue::Servo_StyleSet_FlushStyleSheets]
The problem here is that, when we are evaluating document rule (via Gecko_DocumentRule_UseForPresentation), we invoke the JS engine for the regexp. However, the JS execution is somehow interrupted by InterruptCallback, which instead triggers a force paint, and the force paint flush restyle and lead to this situation.

This is somehow similar to bug 1310335 but in different manner.

Evaluating regexp during parsing might mitigate this issue (because parsing happens far fewer times than restyling), but may not completely fix this issue. We probably need to figure out how can we avoid the regexp evaluation be interrupted.
billm, do you know if it is possible to forbid interruption when we do regexp evaluation inside nsContentUtils::IsPatternMatching [1]?

[1] https://searchfox.org/mozilla-central/rev/cef8389c687203085dc6b52de2fbd0260d7495bf/dom/base/nsContentUtils.cpp#7159-7198
Flags: needinfo?(wmccloskey)
Yes, you can use JS_DisableInterruptCallback and JS_ResetInterruptCallback for this purpose.
Flags: needinfo?(wmccloskey)
Working on a patch given comment 4.
Assignee: nobody → xidorn+moz
Comment on attachment 8886435 [details]
Bug 1379585 part 1 - Add an RAII class for auto disable interrupt callback.

https://reviewboard.mozilla.org/r/157228/#review162416

::: dom/script/ScriptSettings.h:472
(Diff revision 1)
> +class MOZ_RAII AutoDisableJSInterruptCallback
> +{
> +public:
> +  explicit AutoDisableJSInterruptCallback(JSContext* aCx)
> +    : mCx(aCx)
> +    , mOld(JS_DisableInterruptCallback(aCx)) { }

{} in a new line
Attachment #8886435 - Flags: review?(amarchesini) → review+
Comment on attachment 8886436 [details]
Bug 1379585 part 2 - Disable interrupt callback when doing pattern matching.

https://reviewboard.mozilla.org/r/157230/#review162418
Attachment #8886436 - Flags: review?(amarchesini) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6c890fbb9efb
part 1 - Add an RAII class for auto disable interrupt callback. r=baku
https://hg.mozilla.org/integration/autoland/rev/d53368eda554
part 2 - Disable interrupt callback when doing pattern matching. r=baku
https://hg.mozilla.org/mozilla-central/rev/6c890fbb9efb
https://hg.mozilla.org/mozilla-central/rev/d53368eda554
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #14)
> This bug was fixed exactly a month ago.
> 
> There are 7 crashes with this signature since 2017-08-15 04:46:18 again.
> [@ geckoservo::glue::Servo_StyleSet_FlushStyleSheets ]
> 57.0a1: 20170815100349 and 20170814100258
> https://crash-stats.mozilla.com/signature/
> ?signature=geckoservo%3A%3Aglue%3A%3AServo_StyleSet_FlushStyleSheets&date=%3E
> %3D2017-08-02T05%3A53%3A08.000Z&date=%3C2017-08-16T05%3A53%3A08.
> 000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_colum
> ns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-
> date&page=1#reports

I think this deserves another bug, seems like a different stack.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #15)
> (In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #14)
> > This bug was fixed exactly a month ago.
> > 
> > There are 7 crashes with this signature since 2017-08-15 04:46:18 again.
> > [@ geckoservo::glue::Servo_StyleSet_FlushStyleSheets ]
> > 57.0a1: 20170815100349 and 20170814100258
> > https://crash-stats.mozilla.com/signature/
> > ?signature=geckoservo%3A%3Aglue%3A%3AServo_StyleSet_FlushStyleSheets&date=%3E
> > %3D2017-08-02T05%3A53%3A08.000Z&date=%3C2017-08-16T05%3A53%3A08.
> > 000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_colum
> > ns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-
> > date&page=1#reports
> 
> I think this deserves another bug, seems like a different stack.

Huh, all of them seem related to print preview...
See Also: → 1390838
You need to log in before you can comment on or make changes to this bug.