xmlgraphics-fop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 46336] Synchronization fault in FontInfoFinder
Date Mon, 08 Dec 2008 18:19:37 GMT

--- Comment #4 from Andreas L. Delmelle <adelmelle@apache.org>  2008-12-08 10:19:35
PST ---

Investigated this issue closer, and one source of errors seems to be in the
silent assumption, in FontInfoFinder.find(), that the fileLastModified
variable, set right before containsFont() is called, will always be the same as
the cached font file's lastModified() it will be compared to if the call
returns true and getFontInfos() is triggered.
Not even inlining the call to FontCache.getLastModified() will guarantee that,
although it would make occurrences far less likely.
It is conceivable that another thread, in the time between those two calls, has
added a newer version of the file to the cache, which would result in
FontCache.getFontInfos() removing that entry and return null... The file
corresponding to the newly added entry is then reparsed in the original thread
and added again to the map (operation which is thus performed twice, without
good reason)

Besides that, as it was pointed out earlier, only the writes to the Map backing
FontCache are performed in a synchronized fashion. The reads are not, and so
may return results that turn out to be unreliable (i.e. they are guaranteed to
be correct only for the duration of the call).

Trying to get my head around what actually happens there, seems to show at
least /some/ redundancies...
-> fontCache.containsFont()
  -> fontFileMap.containsKey()
-> fontCache.getFontInfos()
  -> fontCache.getFontFile()
    -> fontCache.containsFont()
      -> fontFileMap.containsKey()
    -> fontCache.removeFont()
      -> fontCache.containsFont()
        -> fontFileMap.containsKey()
      -> fontFileMap.remove()
        -> fontFileMap.containsKey() (?)

On the other hand, getFontInfos() itself assumes that getFontFile() will always
return a non-null result, while the implementation a few lines up shows

IMO, to make access by multiple threads easily manageable, FontCache has too
many public methods. In fact, almost none is used anywhere outside the class. 
removeFont() is one obvious candidate for a change in visibility. As for the
interplay between FontInfoFinder and FontCache, it seems that either the one or
the other is assuming too much responsibilities. Can't put my finger yet on
which of the two is stepping out of bounds (initial guess would be the latter),
but I'm definitely going to check if I can clean that up a little.

Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

View raw message