1781831 - Crash in [@ mozilla::StyleCssUrl::GetURI]

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P3)

Description

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

Crash report: https://crash-stats.mozilla.org/report/index/29293321-619e-4674-89b4-b158c0220727

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread())

Top 10 frames of crashing thread:

0 xul.dll mozilla::StyleCssUrl::GetURI const layout/style/ServoStyleConstsInlines.h:430
1 xul.dll static mozilla::dom::FontFaceSetImpl::FindOrCreateUserFontEntryFromFontFace layout/style/FontFaceSetImpl.cpp:512
2 xul.dll mozilla::dom::FontFaceSetImpl::InsertNonRuleFontFace layout/style/FontFaceSetImpl.cpp:322
3 xul.dll mozilla::dom::FontFaceSetWorkerImpl::FlushUserFontSet layout/style/FontFaceSetWorkerImpl.cpp:233
4 xul.dll mozilla::dom::FontFaceImpl::Load layout/style/FontFaceImpl.cpp:301
5 xul.dll mozilla::dom::FontFace::Load layout/style/FontFace.cpp:250
6 xul.dll mozilla::dom::FontFace_Binding::load_promiseWrapper dom/bindings/FontFaceBinding.cpp:1781
7 xul.dll mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ConvertExceptionsToPromises> dom/bindings/BindingUtils.cpp:3285
8 xul.dll Interpret js/src/vm/Interpreter.cpp:3313
9 xul.dll js::RunScript js/src/vm/Interpreter.cpp:389

Comment 1

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

I can reproduce at https://whscience.org/gravityassist/

Comment 2

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

I think this is actually safe because FontFaceImpl::mRule is only ever accessed by its owning thread, and for workers, it lives as long as the FontFaceImpl object itself. It is a little different if created on the main thread for a document, but those are only ever accessed on the main thread anyways.

Comment 3

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

Attachment: Bug 1781831 - Make StyleCssUrl::GetURI assert for main or worker threads.

Ideally we would track the owning thread ID but these classes are
wrappers around the Rust objects.

The servo rule for workers is only accessed on the owning worker thread,
so it should be thread safe to assert either a main or DOM worker
thread.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d2c90caed27
Make StyleCssUrl::GetURI assert for main or worker threads. r=emilio

Comment 5

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/3d2c90caed27