[
https://issues.apache.org/jira/browse/PDFBOX-5056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255944#comment-17255944
]
Tilman Hausherr commented on PDFBOX-5056:
-----------------------------------------
[~mikekap] I'll use the first suggestion in a few minutes. The names "cache"
was introduced in PDFBOX-2262 as part of a huge commit and there's no text why
it was needed.
> Double checked locking done wrong in several places
> ---------------------------------------------------
>
> Key: PDFBOX-5056
> URL: https://issues.apache.org/jira/browse/PDFBOX-5056
> Project: PDFBox
> Issue Type: Bug
> Affects Versions: 2.0.22
> Reporter: Mike Kaplinskiy
> Priority: Major
>
> There's several places inside pdfbox where double-checked locking is done
> wrong. Specifically, double checked locking is the pattern of:
> {code:java}
> private volatile boolean doneInit = false;
> if (!doneInit) {
> synchronized (this) {
> if (!doneInit) {
> ... do init
> doneInit = true;
> }
> }
> }{code}
> Common issues are - lack of {{volatile}} or the volatile set not being last.
> Here are the cases I found so far:
> *
> [https://github.com/apache/pdfbox/blob/e409f25009702be8889ce586b8f6dd7274201f0a/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/color/PDDeviceCMYK.java#L60]
> - {{volatile}} set isn't the last statement in the block.
> *
> [https://github.com/apache/pdfbox/blob/e409f25009702be8889ce586b8f6dd7274201f0a/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/color/PDDeviceRGB.java#L48]
> - {{volatile}} set isn't the last statement in the block.
> *
> [https://github.com/apache/pdfbox/blob/e409f25009702be8889ce586b8f6dd7274201f0a/fontbox/src/main/java/org/apache/fontbox/ttf/TrueTypeFont.java#L162-L167]
> - {{initialized}} isn't {{volatile}}
> *
> [https://github.com/apache/pdfbox/blob/947966ea830ff91c61a6740ca1787eb795b5ca95/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/encoding/Encoding.java#L132-L149]
> - {{names}} isn't {{volatile}} and the second check is missing (which might
> be harmless)
> *
> [https://github.com/apache/pdfbox/blob/6c8526bab8b7ca399721e067d065c1f272f97644/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/Standard14Fonts.java#L172-L190]
> - {{fonts}} isn't even locked and it's a vanilla hashmap that can be
> modified by other threads.
> *
> [https://github.com/apache/pdfbox/blob/7984a52ad4d475886568593614ce566a88bf6bdd/pdfbox/src/main/java/org/apache/pdfbox/cos/COSName.java#L632-L637]
> - (tricky to see, but the constructor adds itself to the map) - the map
> isn't locked before the blind {{put}}, so unclear which invocation "wins"
> (not sure if this is an issue, logic wise).
> *
> [https://github.com/apache/pdfbox/blob/61d6a53eacdee6a40d352509105e1c8d51cfb5dc/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/FontMapperImpl.java#L415-L419]
> - {{fontProvider}} isn't volatile, though is used as a double checked lock
> marker. (Not sure if this an issue, concurrency wise).
> Fixing these one-by-one is possible and what I started doing around
> [https://github.com/apache/pdfbox/pull/90] - but would it make sense to make
> a class that does this properly? Guava has {{Suppliers.memoize}} which does
> the right thing - it's trivial to make an equivalent without the dep in
> pdfbox as well if necessary.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
