[ 
https://issues.apache.org/jira/browse/PDFBOX-4956?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17202627#comment-17202627
 ] 

Andreas Lehmkühler commented on PDFBOX-4956:
--------------------------------------------

I've fixed the hashcode calculation and made the static fields final as 
proposed.

I didn't removed the synchronized from COSName.clearResources() as 
ConcurrentHashMap is thread safe but the access to the elements isn't 
synchronized. A remove operation doesn't look the map so that a concurrent get 
operation may access an element which is deleted right after that.
Excerpt from the class documentation
{code}
 * that key reporting the updated value.)  For aggregate operations
 * such as {@code putAll} and {@code clear}, concurrent retrievals may
 * reflect insertion or removal of only some entries.  Similarly,
{code}

We can't simply replace all COSName occurrencies with String as we need to 
distinguish names (some sort of constant strings) from strings otherwise we 
won't be able to write correct pdfs. Maybe it would make sense to think about 
using strings as key for a COSDictionary instead of COSName. But I'm not sure 
if this give us any real benefit.






> COSName.hashCode initialized after put to cache, instead before
> ---------------------------------------------------------------
>
>                 Key: PDFBOX-4956
>                 URL: https://issues.apache.org/jira/browse/PDFBOX-4956
>             Project: PDFBox
>          Issue Type: Bug
>          Components: Parsing
>    Affects Versions: 2.0.21
>            Reporter: Jörg Waßmer
>            Assignee: Andreas Lehmkühler
>            Priority: Major
>             Fix For: 2.0.22, 3.0.0 PDFBox
>
>
> In the constructor org.apache.pdfbox.cos.COSName.COSName(String, boolean), 
> the field COSNam.hashCode becomes initialized after the COSName instance has 
> been added to the cache.
> Thus, concurrent threads using the cached instance may see different values 
> in COSName.hashCode().
>  
> Just to mention, that's another problem:
>  The whole caching is quite dirty, because it leaks memory if the application 
> is not aware of calling COSName.clearResources().
>  Ideally, the class COSName would not exist at all, since it has no benefit 
> over using strings directly. Of course, it would be quite a hard work to get 
> rid of it.
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to