Bugzilla 46211:

Fixed some multi-threading issues in FontCache.java (+ some minor cleanup: simplification of conditionals)
Thanks to rogov.AT.devexperts.com for tracing and reporting the problem.

git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@718309 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/java/org/apache/fop/fonts/FontCache.java b/src/java/org/apache/fop/fonts/FontCache.java
index 2528989bbbdf83050de2e53f01bbd5881e3869f2..a3ff1b672f203564fdb3eae2dcf3131fc7016efe 100644 (file)
--- a/src/java/org/apache/fop/fonts/FontCache.java
+++ b/src/java/org/apache/fop/fonts/FontCache.java
@@ -48,7 +48,7 @@ public final class FontCache implements Serializable {
      * Serialization Version UID. Change this value if you want to make sure the user's cache
      * file is purged after an update.
      */
-    private static final long serialVersionUID = 605232520271754718L;
+    private static final long serialVersionUID = 605232520271754719L;
 
     /** logging instance */
     private static Log log = LogFactory.getLog(FontCache.class);
@@ -64,7 +64,7 @@ public final class FontCache implements Serializable {
     private transient boolean changed = false;
 
     /** change lock */
-    private transient Object changeLock = new Object();
+    private final Object changeLock = new Object();
 
     /** master mapping of font url -> font info.  This needs to be
      *  a list, since a TTC file may contain more than 1 font. */
@@ -80,12 +80,6 @@ public final class FontCache implements Serializable {
         //nop
     }
 
-    private void readObject(java.io.ObjectInputStream in)
-            throws IOException, ClassNotFoundException {
-        in.defaultReadObject();
-        this.changeLock = new Object(); //Initialize transient field
-    }
-
     private static File getUserHome() {
         String s = System.getProperty("user.home");
         if (s != null) {
@@ -226,10 +220,8 @@ public final class FontCache implements Serializable {
      * @return boolean
      */
     public boolean containsFont(String embedUrl) {
-        if (embedUrl != null) {
-            return getFontFileMap().containsKey(embedUrl);
-        }
-        return false;
+        return (embedUrl != null
+                && getFontFileMap().containsKey(embedUrl));
     }
 
     /**
@@ -238,10 +230,8 @@ public final class FontCache implements Serializable {
      * @return font
      */
     public boolean containsFont(EmbedFontInfo fontInfo) {
-        if (fontInfo != null) {
-            return getFontFileMap().containsKey(getCacheKey(fontInfo));
-        }
-        return false;
+        return (fontInfo != null
+                && getFontFileMap().containsKey(getCacheKey(fontInfo)));
     }
 
     /**
@@ -316,10 +306,7 @@ public final class FontCache implements Serializable {
      * @return CachedFontFile object
      */
     public CachedFontFile getFontFile(String embedUrl) {
-        if (containsFont(embedUrl)) {
-            return (CachedFontFile)getFontFileMap().get(embedUrl);
-        }
-        return null;
+        return containsFont(embedUrl) ? (CachedFontFile) getFontFileMap().get(embedUrl) : null;
     }
 
     /**
@@ -362,8 +349,8 @@ public final class FontCache implements Serializable {
      * @return whether this is a failed font
      */
     public boolean isFailedFont(String embedUrl, long lastModified) {
-        if (getFailedFontMap().containsKey(embedUrl)) {
-            synchronized (changeLock) {
+        synchronized (changeLock) {
+            if (getFailedFontMap().containsKey(embedUrl)) {
                 long failedLastModified = ((Long)getFailedFontMap().get(embedUrl)).longValue();
                 if (lastModified != failedLastModified) {
                     // this font has been changed so lets remove it
@@ -371,10 +358,11 @@ public final class FontCache implements Serializable {
                     getFailedFontMap().remove(embedUrl);
                     changed = true;
                 }
+                return true;
+            } else {
+                return false;
             }
-            return true;
         }
-        return false;
     }
 
     /**
@@ -457,10 +445,8 @@ public final class FontCache implements Serializable {
         }
 
         public boolean containsFont(EmbedFontInfo efi) {
-            if (efi.getPostScriptName() != null) {
-                return getFileFontsMap().containsKey(efi.getPostScriptName());
-            }
-            return false;
+            return efi.getPostScriptName() != null
+                    && getFileFontsMap().containsKey(efi.getPostScriptName());
         }
 
         public EmbedFontInfo[] getEmbedFontInfos() {

diff --git a/status.xml b/status.xml
index 85cd319353ccf7d77873090052c06a18e0b9bfb2..2197bc0057b62164dfaa784684da0585dce5da25 100644 (file)
--- a/status.xml
+++ b/status.xml
@@ -53,6 +53,15 @@
 
   <changes>
     <release version="FOP Trunk" date="TBD">
+      <action context="Code" dev="AD" type="fix" fixes-bug="46211" due-to="rogov.AT.devexperts.com">
+        Fixed some multi-threading issues in FontCache.java:
+        <ul>
+        <li>remove the unused private readObject()</li>
+        <li>make the changeLock member final (initialization-safety + impossible to reassign)</li>
+        <li>perform the HashMap check for a failed font inside the synchronized block to avoid a race condition</li>
+        </ul>
+      </action>
+      </action>
       <action context="Renderers" dev="AD" type="add" fixes-bug="45113" due-to="Alexander Stamenov">
         Added PDF /Launch action: references to URIs using the file:// protocol will now generate
         a /Launch action as opposed to a more general /URI (which doesn't yield the expected result