Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove LCMS mutex #53

Closed
deymo opened this issue May 31, 2021 · 2 comments · Fixed by #112
Closed

Remove LCMS mutex #53

deymo opened this issue May 31, 2021 · 2 comments · Fixed by #112
Labels

Comments

@deymo
Copy link
Contributor

deymo commented May 31, 2021

Describe the bug
We have a mutex in the LCMS color_management which can probably be removed after an update in LCMS. See #23 (comment) for context.

@deymo deymo added the cleanup label May 31, 2021
@kleisauke
Copy link
Contributor

I tried to remove that mutex on this branch, but it seems that LCMS still has a thread-safety issue regarding gmtime.
https://github.com/mm2/Little-CMS/blob/807809d7dd9b09c0380b71d5d5ebee3c1439ca99/src/cmsio0.c#L503-L504

TSAN results

Tested branch: main...kleisauke:remove-lcms-mutex

Run with:

$ BUILD_DIR=build_tsan CC=clang-12 CXX=clang++-12 SKIP_TEST=1 ./ci.sh tsan -DJPEGXL_ENABLE_SKCMS=OFF
$ ./build_tsan/lib/tests/passes_test

Commit kleisauke@28c3009:

ThreadSanitizer: reported 8 warnings
(see tsan-lcms2.10.log for more details)

Commit kleisauke@e1a97c9:

ThreadSanitizer: reported 2 warnings
(see tsan-lcms2.12.log for more details)

Attachments:
tsan-lcms2.10.log
tsan-lcms2.12.log

@kleisauke
Copy link
Contributor

A pull request has been made to fix the gmtime issue at mm2/Little-CMS#263.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants