[
https://issues.apache.org/jira/browse/PDFBOX-5056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17255866#comment-17255866
]
Tilman Hausherr commented on PDFBOX-5056:
-----------------------------------------
I just tried the "bad" code from my previous comment and no deadlock. This is
bad because it means we can no longer reproduce the problem. Maybe because of
newer jdk, or faster computer.
> 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: [email protected]
For additional commands, e-mail: [email protected]
