JuliaGraphics / FreeTypeAbstraction.jl

A Julian abstraction layer over FreeType.jl
Other
25 stars 20 forks source link

Add locking mechanism around FT_New_Face and FT_Done_Face #78

Closed jkrumbiegel closed 5 months ago

jkrumbiegel commented 1 year ago

FreeType has been showing up in threading issues when using Makie. When checking the FreeType docs, I found that when using a single library object, one should lock on FT_New_Face and FT_Done_Face, which is implemented here. I tried to follow the best practices for locks in finalizers and took the implementation from this PR https://github.com/JuliaIO/HDF5.jl/pull/1049/files which got it from here https://docs.julialang.org/en/v1/manual/multi-threading/#Safe-use-of-Finalizers

I also added an ENV variable setting the number of threads in CI tests to 4 so that the test (which is very basic and could probably be improved) should have multiple threads to run on. The test doesn't ensure that, but I didn't quickly find an easy way to do it except the unmaintained (by now probably outdated) library https://github.com/JuliaTesting/PerformanceTestTools.jl

This does not solve the problem of using FT_Face objects from multiple threads but I'll leave that for a future implementation. Probably, those objects should each get their own lock.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 75.00% and project coverage change: +0.11 :tada:

Comparison is base (b7d6d65) 94.62% compared to head (bfc60dc) 94.73%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #78 +/- ## ========================================== + Coverage 94.62% 94.73% +0.11% ========================================== Files 6 6 Lines 316 323 +7 ========================================== + Hits 299 306 +7 Misses 17 17 ``` | [Impacted Files](https://codecov.io/gh/JuliaGraphics/FreeTypeAbstraction.jl/pull/78?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics) | Coverage Δ | | |---|---|---| | [src/types.jl](https://codecov.io/gh/JuliaGraphics/FreeTypeAbstraction.jl/pull/78?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics#diff-c3JjL3R5cGVzLmps) | `92.72% <75.00%> (-1.51%)` | :arrow_down: | ... and [3 files with indirect coverage changes](https://codecov.io/gh/JuliaGraphics/FreeTypeAbstraction.jl/pull/78/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

jkrumbiegel commented 5 months ago

superseded by https://github.com/JuliaGraphics/FreeTypeAbstraction.jl/pull/87