1756474 - Make the platform font list usable from worker threads

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jonathan Kew [:jfkthame]]

gfxPlatformFontList and the objects it manages (font family and face records) is largely read, not written, but various pieces of the data are populated lazily as additional fonts get used. So to allow multiple threads to use the font list, we need locking around access/updates.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1756474 - Add locking to gfxPlatformFontList and gfxFontFamily, to allow use from worker threads. r=lsalzman

Some of the gfxPlatformFontList methods are a bit tricky in that they may be called
either during reflow (e.g. through gfxTextRun, gfxFontEntry, etc), in which case
they need to lock internally before accessing various members; but they may also
be used during font-list initialization or other times when the lock is already
being held. In that case, they must NOT attempt to lock internally, or we'll hit
a deadlock.

In some cases this is easily handled by exposing both a Foo and a FooLocked version
of the method, and directly calling the Locked version if we're already holding the
mutex. But the calling code may itself be used in different contexts, sometimes with
the font-list already locked and sometimes not. E.g. a method like gfxFontEntry::ReadCMAP,
which may be used during font-list initialization (lock already held), or during CSS
font-matching (lock not held). In cases like this, rather than creating parallel
"Locked" copies of the possible intervening methods, we pass a FontListLocked flag
through the various methods so that when we get back to gfxPlatformFontList code at
the bottom, we can tell whether we need to grab the mutex or if we already hold it.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/335840401d5e
Add locking to gfxPlatformFontList and gfxFontFamily, to allow use from worker threads. r=lsalzman

Comment 3

[Cristian Tuns]

Backed out for causing multiple failures.

• Backout link
• Push with failures
• Failure Log1
• Failure Log2
• Failure Log3
• Failure line: SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock) /builds/worker/checkouts/gecko/mozglue/misc/RWLock_posix.cpp:38:3 in mozilla::detail::RWLockImpl::readLock()

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a38c915b806d
Add locking to gfxPlatformFontList and gfxFontFamily, to allow use from worker threads. r=lsalzman

Comment 5

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/a38c915b806d