1958121 - ThreadSanitizer: data race [@ gfxFont::CheckForFeaturesInvolvingSpace] vs. [@ gfxFont::CheckForFeaturesInvolvingSpace]

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith] (ooo)]

Attachment: tsan-log.txt

Found with m-c 20250402-6be6a06991d7 (--enable-thread-sanitizer)

This was found by visiting a live website with a TSan build.

STR:

• Launch browser and visit: http://vologda-poisk.ru/

Or via site-scout:

$ pip install fuzzfetch site-scout --upgrade
$ fuzzfetch -t -n firefox
$ site-scout ./firefox/firefox --explore -u <url>

WARNING: ThreadSanitizer: data race (pid=132724)
  Write of size 4 at 0x7254001a5a00 by thread T33 (mutexes: write M0):
    #0 gfxFont::CheckForFeaturesInvolvingSpace() const /builds/worker/checkouts/gecko/gfx/thebes/gfxFont.cpp (libxul.so+0x5095831) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #1 gfxFont::SpaceMayParticipateInShaping(mozilla::intl::Script) const /builds/worker/checkouts/gecko/gfx/thebes/gfxFont.cpp:1534:5 (libxul.so+0x5095c7b) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #2 bool gfxFont::SplitAndInitTextRun<char16_t>(mozilla::gfx::DrawTarget*, gfxTextRun*, char16_t const*, unsigned int, unsigned int, mozilla::intl::Script, nsAtom*, mozilla::gfx::ShapedTextFlags) /builds/worker/checkouts/gecko/gfx/thebes/gfxFont.cpp:3689:7 (libxul.so+0x50a789d) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #3 void gfxFontGroup::InitScriptRun<char16_t>(mozilla::gfx::DrawTarget*, gfxTextRun*, char16_t const*, unsigned int, unsigned int, mozilla::intl::Script, gfxMissingFontRecorder*) /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:2863:25 (libxul.so+0x512c387) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #4 void gfxFontGroup::InitTextRun<char16_t>(mozilla::gfx::DrawTarget*, gfxTextRun*, char16_t const*, unsigned int, gfxMissingFontRecorder*) /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp (libxul.so+0x51186cd) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #5 already_AddRefed<gfxTextRun> gfxFontGroup::MakeTextRun<char16_t>(char16_t const*, unsigned int, gfxTextRunFactory::Parameters const*, mozilla::gfx::ShapedTextFlags, nsTextFrameUtils::Flags, gfxMissingFontRecorder*) /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:2544:3 (libxul.so+0x51180df) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #6 MakeTextRun<char16_t> /builds/worker/workspace/obj-build/dist/include/gfxTextRun.h:983:12 (libxul.so+0x6c64e5e) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #7 mozilla::dom::CanvasBidiProcessor::SetText(char16_t const*, int, mozilla::intl::BidiDirection) /builds/worker/checkouts/gecko/dom/canvas/CanvasRenderingContext2D.cpp:4593:26 (libxul.so+0x6c64e5e)
    #8 nsBidiPresUtils::ProcessSimpleRun(char16_t const*, unsigned long, mozilla::intl::BidiEmbeddingLevel, nsPresContext*, nsBidiPresUtils::BidiProcessor&, nsBidiPresUtils::Mode, nsBidiPositionResolve*, int, int*) /builds/worker/checkouts/gecko/layout/base/nsBidiPresUtils.cpp:2418:14 (libxul.so+0x9462d41) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #9 nsBidiPresUtils::ProcessText(char16_t const*, unsigned long, mozilla::intl::BidiEmbeddingLevel, nsPresContext*, nsBidiPresUtils::BidiProcessor&, nsBidiPresUtils::Mode, nsBidiPositionResolve*, int, int*, mozilla::intl::Bidi&) /builds/worker/checkouts/gecko/layout/base/nsBidiPresUtils.cpp:2191:5 (libxul.so+0x9462b15) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #10 mozilla::dom::CanvasRenderingContext2D::DrawOrMeasureText(nsTSubstring<char16_t> const&, float, float, mozilla::dom::Optional<double> const&, mozilla::dom::CanvasRenderingContext2D::TextDrawOperation, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/canvas/CanvasRenderingContext2D.cpp:4997:12 (libxul.so+0x6be1fba) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    ...

  Previous write of size 4 at 0x7254001a5a00 by main thread:
    #0 gfxFont::CheckForFeaturesInvolvingSpace() const /builds/worker/checkouts/gecko/gfx/thebes/gfxFont.cpp (libxul.so+0x5095831) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #1 gfxFont::SpaceMayParticipateInShaping(mozilla::intl::Script) const /builds/worker/checkouts/gecko/gfx/thebes/gfxFont.cpp:1534:5 (libxul.so+0x5095c7b) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #2 bool gfxFont::SplitAndInitTextRun<unsigned char>(mozilla::gfx::DrawTarget*, gfxTextRun*, unsigned char const*, unsigned int, unsigned int, mozilla::intl::Script, nsAtom*, mozilla::gfx::ShapedTextFlags) /builds/worker/checkouts/gecko/gfx/thebes/gfxFont.cpp:3689:7 (libxul.so+0x50a3f67) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #3 void gfxFontGroup::InitScriptRun<unsigned char>(mozilla::gfx::DrawTarget*, gfxTextRun*, unsigned char const*, unsigned int, unsigned int, mozilla::intl::Script, gfxMissingFontRecorder*) /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:2863:25 (libxul.so+0x512d6bf) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #4 void gfxFontGroup::InitTextRun<unsigned char>(mozilla::gfx::DrawTarget*, gfxTextRun*, unsigned char const*, unsigned int, gfxMissingFontRecorder*) /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:2772:9 (libxul.so+0x511776c) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #5 already_AddRefed<gfxTextRun> gfxFontGroup::MakeTextRun<unsigned char>(unsigned char const*, unsigned int, gfxTextRunFactory::Parameters const*, mozilla::gfx::ShapedTextFlags, nsTextFrameUtils::Flags, gfxMissingFontRecorder*) /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:2544:3 (libxul.so+0x5117172) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #6 BuildTextRunsScanner::BuildTextRunForFrames(void*) /builds/worker/checkouts/gecko/layout/generic/nsTextFrame.cpp:2583:28 (libxul.so+0x968bb03) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #7 BuildTextRunsScanner::FlushFrames(bool, bool) /builds/worker/checkouts/gecko/layout/generic/nsTextFrame.cpp:1671:17 (libxul.so+0x968853f) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #8 BuildTextRuns /builds/worker/checkouts/gecko/layout/generic/nsTextFrame.cpp:1590:11 (libxul.so+0x96913a1) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #9 nsTextFrame::EnsureTextRun(nsTextFrame::TextRunType, mozilla::gfx::DrawTarget*, nsIFrame*, GenericLineListIterator<nsLineLink, false> const*, unsigned int*) /builds/worker/checkouts/gecko/layout/generic/nsTextFrame.cpp:3058:7 (libxul.so+0x96913a1)
    #10 nsTextFrame::AddInlineMinISizeForFlow(gfxContext*, nsIFrame::InlineMinISizeData*, nsTextFrame::TextRunType) /builds/worker/checkouts/gecko/layout/generic/nsTextFrame.cpp:8725:7 (libxul.so+0x96b135c) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    ...

  Location is heap block of size 544 at 0x7254001a5900 allocated by main thread:
    #0 malloc /builds/worker/fetches/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:666:5 (firefox-bin+0xd58c0) (BuildId: 92681cc6fb315a0bf36ded985fae788eda2db99d)
    #1 moz_xmalloc /builds/worker/checkouts/gecko/memory/mozalloc/mozalloc.cpp:52:15 (firefox-bin+0x15cd08) (BuildId: 92681cc6fb315a0bf36ded985fae788eda2db99d)
    #2 operator new /builds/worker/workspace/obj-build/dist/include/mozilla/cxxalloc.h:33:10 (libxul.so+0x5052e8d) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #3 gfxFcPlatformFontList::CreateFontEntry(mozilla::fontlist::Face*, mozilla::fontlist::Family const*) /builds/worker/checkouts/gecko/gfx/thebes/gfxFcPlatformFontList.cpp:2126:14 (libxul.so+0x5052e8d)
    #4 operator() /builds/worker/checkouts/gecko/gfx/thebes/gfxPlatformFontList.cpp:1938:40 (libxul.so+0x50f665c) (BuildId: 1b4df73d2960b11e33c906a55c0fde3130b971b8)
    #5 OrInsertWith<(lambda at /builds/worker/checkouts/gecko/gfx/thebes/gfxPlatformFontList.cpp:1938:27)> /builds/worker/workspace/obj-build/dist/include/nsBaseHashtable.h:739:23 (libxul.so+0x50f665c)
    #6 operator()<nsBaseHashtable<nsPtrHashKey<const mozilla::fontlist::Face>, RefPtr<gfxFontEntry>, gfxFontEntry *, nsDefaultConverter<RefPtr<gfxFontEntry>, gfxFontEntry *> >::EntryHandle> /builds/worker/workspace/obj-build/dist/include/nsBaseHashtable.h:436:26 (libxul.so+0x50f665c)
    #7 operator()<nsTHashtable<nsBaseHashtableET<nsPtrHashKey<const mozilla::fontlist::Face>, RefPtr<gfxFontEntry> > >::EntryHandle> /builds/worker/workspace/obj-build/dist/include/nsBaseHashtable.h:849:18 (libxul.so+0x50f665c)
    #8 operator()<PLDHashTable::EntryHandle> /builds/worker/workspace/obj-build/dist/include/nsTHashtable.h:439:18 (libxul.so+0x50f665c)
    #9 WithEntryHandle<(lambda at /builds/worker/workspace/obj-build/dist/include/nsTHashtable.h:438:9)> /builds/worker/workspace/obj-build/dist/include/PLDHashTable.h:605:12 (libxul.so+0x50f665c)
    #10 WithEntryHandle<(lambda at /builds/worker/workspace/obj-build/dist/include/nsBaseHashtable.h:848:15)> /builds/worker/workspace/obj-build/dist/include/nsTHashtable.h:436:25 (libxul.so+0x50f665c)
    ...

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:lsalzman, could you have a look please?

For more information, please visit BugBot documentation.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1958121 - Use exchange to update the SpaceFeatures flags.

Comment 3

[Jonathan Kew [:jfkthame]]

Tyson, any chance you could check whether the attached patch resolves this? AFAICS from inspection, I'm guessing this is the problem. I was going to try and verify it locally, but my TSan build is currently broken. :-(

Comment 4

[Tyson Smith [:tsmith] (ooo)]

I was not able to reproduce the issue with the latest build, maybe some content changed?

Comment 5

[Jonathan Kew [:jfkthame]]

(In reply to Tyson Smith [:tsmith] from comment #4)

I was not able to reproduce the issue with the latest build, maybe some content changed?

OK, thanks for checking. I'll go ahead and land the patch anyhow, as it looks correct in principle, even if we can't currently confirm that this was exactly the original issue here.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e47c8c303da5
Use exchange to update the SpaceFeatures flags. r=lsalzman

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Please create an ESR128 uplift request for this when you get a chance.

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1958121 - Use exchange to update the SpaceFeatures flags.

Original Revision: https://phabricator.services.mozilla.com/D245913

Comment 10

[Phabricator Automation]

firefox-esr128 Uplift Approval Request

• User impact if declined: possible data race - user impact unclear
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: (no known STR)
• Risk associated with taking this patch: minimal
• Explanation of risk level: trivial patch to safely update atomic variable
• String changes made/needed: none
• Is Android affected?: yes

Comment 11

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/5e9ebb5ba1f1