1915433 - gfxPlatform::EnsureCMSInitialized leads to presumably harmless reentrancy in gfxPlatform::InitializeCMS

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Yannis Juglaret [:yannis]]

While investigating a crash in qcms::transform_avx::qcms_transform_data_template_lut_avx (for which I will file a different bug), I followed the initialization of gCMSBGRATransform and realized that it is getting initialized twice.

Call stack for first initialization:

02 xul!gfxPlatform::InitializeCMS+0x16d [/builds/worker/checkouts/gecko/gfx/thebes/gfxPlatform.cpp @ 2186]
03 xul!gfxPlatform::EnsureCMSInitialized+0x230 [/builds/worker/checkouts/gecko/gfx/thebes/gfxPlatform.h @ 952]
04 xul!gfxPlatform::GetCMSMode+0x230 [/builds/worker/checkouts/gecko/gfx/thebes/gfxPlatform.h @ 499]
05 xul!nsXPLookAndFeel::GetUncachedColor+0x2ee [/builds/worker/checkouts/gecko/widget/nsXPLookAndFeel.cpp @ 1025]
06 xul!nsXPLookAndFeel::GetColorValue+0x399 [/builds/worker/checkouts/gecko/widget/nsXPLookAndFeel.cpp @ 1005]
07 xul!mozilla::LookAndFeel::GetColor+0x3d0 [/builds/worker/checkouts/gecko/widget/nsXPLookAndFeel.cpp @ 1398]
...

Call stack for second initialization:

02 xul!gfxPlatform::InitializeCMS+0x16d [/builds/worker/checkouts/gecko/gfx/thebes/gfxPlatform.cpp @ 2186]
03 xul!gfxPlatform::Init+0x906 [/builds/worker/checkouts/gecko/gfx/thebes/gfxPlatform.cpp @ 980]
04 xul!gfxPlatform::GetPlatform+0x26 [/builds/worker/checkouts/gecko/gfx/thebes/gfxPlatform.cpp @ 464]
05 xul!gfxPlatform::InitializeCMS+0x1c0 [/builds/worker/checkouts/gecko/gfx/thebes/gfxPlatform.cpp @ 2137]
06 xul!gfxPlatform::EnsureCMSInitialized+0x230 [/builds/worker/checkouts/gecko/gfx/thebes/gfxPlatform.h @ 952]
07 xul!gfxPlatform::GetCMSMode+0x230 [/builds/worker/checkouts/gecko/gfx/thebes/gfxPlatform.h @ 499]
08 xul!nsXPLookAndFeel::GetUncachedColor+0x2ee [/builds/worker/checkouts/gecko/widget/nsXPLookAndFeel.cpp @ 1025]
09 xul!nsXPLookAndFeel::GetColorValue+0x399 [/builds/worker/checkouts/gecko/widget/nsXPLookAndFeel.cpp @ 1005]
0a xul!mozilla::LookAndFeel::GetColor+0x3d0 [/builds/worker/checkouts/gecko/widget/nsXPLookAndFeel.cpp @ 1398]
...

This occurs because gfxPlatform::EnsureCMSInitialized can call gfxPlatform::InitializeCMS before gPlatform is initialized. In this case when gfxPlatform::InitializeCMS calls gfxPlatform::GetPlatform, it leads to a reentrant call to gfxPlatform::InitializeCMS from within gfxPlatform::Init. The original call and the reentrant call will do redundant work - in particular gCMSBGRATransform gets initialized twice.

The reentrancy looks mostly harmless (just initializing some stuff twice) but nevertheless looks unintended and undesirable. It feels like either gfxPlatform::InitializeCMS should not rely on gfxPlatform::GetPlatform, or gfxPlatform::EnsureCMSInitialized should call the whole gfxPlatform::Init directly rather than just gfxPlatform::InitializeCMS.

Edit: I inverted the first and second initialization call stacks in my original message.

Comment 1

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

:emilio, since you are the author of the regressor, bug 1678487, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1915433 - Simplify CMS and gfxPlatform initialization. r=#gfx-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03a2926ea510
Simplify CMS and gfxPlatform initialization. r=gfx-reviewers,bradwerth

Comment 4

[tszentpeteri]

Backed out for causing mochitest assertion failures on gfxPlatform.cpp

• Backout link
• Push with failures
• Failure Log mochitest
• Failure Log reftest
• Failure Log wpt
• Failure Log mochitest 2:

[task 2024-08-28T22:38:32.919Z] 22:38:32     INFO -  TEST-START | layout/style/test/test_animations_omta.html
[task 2024-08-28T22:45:16.786Z] 22:45:16     INFO -  wait for org.mozilla.geckoview.test_runner complete; top activity=org.mozilla.geckoview.test_runner
[task 2024-08-28T22:45:16.786Z] 22:45:16     INFO -  org.mozilla.geckoview.test_runner unexpectedly found running. Killing...
[task 2024-08-28T22:45:29.991Z] 22:45:29  WARNING -  TEST-UNEXPECTED-FAIL | layout/style/test/test_animations_omta.html | application timed out after 370 seconds with no output
[task 2024-08-28T22:45:29.991Z] 22:45:29     INFO -  runtestsremote.py | Application ran for: 0:07:11.818089
[task 2024-08-28T22:45:30.268Z] 22:45:30     INFO -  runtests.py | Running with scheme: https
[task 2024-08-28T22:45:30.268Z] 22:45:30     INFO -  runtests.py | Running with e10s: True
[task 2024-08-28T22:45:30.268Z] 22:45:30     INFO -  runtests.py | Running with fission: False
[task 2024-08-28T22:45:30.268Z] 22:45:30     INFO -  runtests.py | Running with cross-origin iframes: False
[task 2024-08-28T22:45:30.268Z] 22:45:30     INFO -  runtests.py | Running with serviceworker_e10s: True
[task 2024-08-28T22:45:30.268Z] 22:45:30     INFO -  runtests.py | Running with socketprocess_e10s: False
[task 2024-08-28T22:45:30.268Z] 22:45:30     INFO -  runtests.py | Running tests: start.
[task 2024-08-28T22:45:30.358Z] 22:45:30     INFO -  deleted remote log /data/local/tmp/test_root/logs/mochitest.log
[task 2024-08-28T22:45:30.387Z] 22:45:30     INFO -  adb Granting important runtime permissions to org.mozilla.geckoview.test_runner
[task 2024-08-28T22:45:31.510Z] 22:45:31     INFO -  adb launch_application: am start -W -n org.mozilla.geckoview.test_runner/org.mozilla.geckoview.test_runner.TestRunnerActivity -a android.intent.action.MAIN --es env0 MOZ_CRASHREPORTER_NO_REPORT=1 --es env1 MOZ_CRASHREPORTER=1 --es env2 MOZ_CRASHREPORTER_SHUTDOWN=1 --es env3 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es env4 MOZ_IN_AUTOMATION=1 --es env5 R_LOG_LEVEL=6 --es env6 R_LOG_DESTINATION=stderr --es env7 R_LOG_VERBOSE=1 --es env8 XPCOM_DEBUG_BREAK=stack --es env9 MOZ_UPLOAD_DIR=/data/local/tmp/test_root/mozlog --es env10 MOZ_HIDE_RESULTS_TABLE=1 --es arg0 -profile --es arg1 /data/local/tmp/test_root/profile/ -d 'https://example.com:443/tests?autorun=1&closeWhenDone=1&logFile=%2Fdata%2Flocal%2Ftmp%2Ftest_root%2Flogs%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO&hideResultsTable=1&manifestFile=tests.json&dumpOutputDirectory=%2Fdata%2Flocal%2Ftmp%2Ftest_root&ignorePrefsFile=ignorePrefs.json'
[task 2024-08-28T22:45:33.028Z] 22:45:33     INFO -  runtestsremote.py | Application pid: 5519
[task 2024-08-28T22:45:35.200Z] 22:45:35     INFO -  TEST-START | layout/style/test/test_bug1443344-1.html

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef158dc7becf
Simplify CMS and gfxPlatform initialization. r=gfx-reviewers,bradwerth

Comment 6

[Serban Stanca [:SerbanS]]

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

Comment 7

[Serban Stanca [:SerbanS]]

Backed out for causing xpcshell failures.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-TIMEOUT | tools/profiler/tests/xpcshell/test_feature_cpufreq.js | Test timed out

Hi Emilio! I decided to back out this changeset because this problem also appeared in the meantime on the tsan opt build. Please also be aware about these xpcshell failures.

• Failure Log

Thank you!

Comment 8

[tszentpeteri]

Backout merged to central:
https://hg.mozilla.org/mozilla-central/rev/459ab7d6c83504b10af1efa4db40570a9e1ccaa1

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a171245a2785
Simplify CMS and gfxPlatform initialization. r=gfx-reviewers,bradwerth

Comment 10

[Cristian Tuns]

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