Mike Kaplinskiy created PDFBOX-5056:
---------------------------------------
Summary: 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
Reporter: Mike Kaplinskiy
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]
