diff options
-rw-r--r-- | src/java/org/apache/fop/fo/flow/ExternalGraphic.java | 19 | ||||
-rw-r--r-- | src/java/org/apache/fop/image/ImageFactory.java | 65 | ||||
-rw-r--r-- | status.xml | 9 |
3 files changed, 72 insertions, 21 deletions
diff --git a/src/java/org/apache/fop/fo/flow/ExternalGraphic.java b/src/java/org/apache/fop/fo/flow/ExternalGraphic.java index d80cbe868..a84256b46 100644 --- a/src/java/org/apache/fop/fo/flow/ExternalGraphic.java +++ b/src/java/org/apache/fop/fo/flow/ExternalGraphic.java @@ -41,7 +41,8 @@ public class ExternalGraphic extends AbstractGraphics { //Additional values private String url; - private FopImage fopimage; + private int intrinsicWidth; + private int intrinsicHeight; /** * Create a new External graphic node. @@ -63,7 +64,7 @@ public class ExternalGraphic extends AbstractGraphics { url = ImageFactory.getURL(getSrc()); FOUserAgent userAgent = getUserAgent(); ImageFactory fact = userAgent.getFactory().getImageFactory(); - fopimage = fact.getImage(url, userAgent); + FopImage fopimage = fact.getImage(url, userAgent); if (fopimage == null) { getLogger().error("Image not available: " + getSrc()); } else { @@ -71,6 +72,8 @@ public class ExternalGraphic extends AbstractGraphics { if (!fopimage.load(FopImage.DIMENSIONS)) { getLogger().error("Cannot read image dimensions: " + getSrc()); } + this.intrinsicWidth = fopimage.getIntrinsicWidth(); + this.intrinsicHeight = fopimage.getIntrinsicHeight(); } //TODO Report to caller so he can decide to throw an exception } @@ -122,22 +125,14 @@ public class ExternalGraphic extends AbstractGraphics { * @see org.apache.fop.fo.flow.AbstractGraphics#getIntrinsicWidth() */ public int getIntrinsicWidth() { - if (fopimage != null) { - return fopimage.getIntrinsicWidth(); - } else { - return 0; - } + return this.intrinsicWidth; } /** * @see org.apache.fop.fo.flow.AbstractGraphics#getIntrinsicHeight() */ public int getIntrinsicHeight() { - if (fopimage != null) { - return fopimage.getIntrinsicHeight(); - } else { - return 0; - } + return this.intrinsicHeight; } } diff --git a/src/java/org/apache/fop/image/ImageFactory.java b/src/java/org/apache/fop/image/ImageFactory.java index 376dc6686..acf310ada 100644 --- a/src/java/org/apache/fop/image/ImageFactory.java +++ b/src/java/org/apache/fop/image/ImageFactory.java @@ -20,6 +20,9 @@ package org.apache.fop.image; // Java import java.io.InputStream; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.SoftReference; import java.lang.reflect.Constructor; import java.util.ArrayList; import java.util.Map; @@ -28,6 +31,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map.Entry; + import javax.xml.transform.Source; import javax.xml.transform.stream.StreamSource; @@ -39,7 +44,6 @@ import org.apache.fop.image.analyser.ImageReaderFactory; import org.apache.fop.apps.FOUserAgent; import org.apache.fop.datatypes.URISpecification; - /** * Create FopImage objects (with a configuration file - not yet implemented). * @author Eric SCHAEFFER @@ -99,6 +103,8 @@ public final class ImageFactory { imt = new ImageMimeType("image/png"); imageMimeTypes.put(imt.getMimeType(), imt); + //Image I/O is faster and more memory-efficient than own codec for PNG + imt.addProvider(imageIoImage); imt.addProvider(pngImage); imt = new ImageMimeType("image/tga"); @@ -109,8 +115,9 @@ public final class ImageFactory { imt = new ImageMimeType("image/tiff"); imageMimeTypes.put(imt.getMimeType(), imt); - imt.addProvider(tiffImage); - imt.addProvider(jaiImage); + imt.addProvider(tiffImage); //Slower but supports CCITT embedding + imt.addProvider(imageIoImage); //Fast but doesn't support CCITT embedding + imt.addProvider(jaiImage); //Fast but doesn't support CCITT embedding imt = new ImageMimeType("image/svg+xml"); imageMimeTypes.put(imt.getMimeType(), imt); @@ -357,12 +364,13 @@ class ContextImageCache implements ImageCache { private boolean collective; private Map contextStore = Collections.synchronizedMap(new java.util.HashMap()); private Set invalid = null; - private Map weakStore = null; + private Map refStore = null; + private ReferenceQueue refQueue = new ReferenceQueue(); public ContextImageCache(boolean col) { collective = col; if (collective) { - weakStore = Collections.synchronizedMap(new java.util.WeakHashMap()); + refStore = Collections.synchronizedMap(new java.util.HashMap()); invalid = Collections.synchronizedSet(new java.util.HashSet()); } } @@ -399,7 +407,14 @@ class ContextImageCache implements ImageCache { } } if (im == null) { - im = (ImageLoader) weakStore.get(url); + Reference ref = (Reference)refStore.get(url); + if (ref != null) { + im = (ImageLoader) ref.get(); + if (im == null) { + //Remove key if its value has been garbage collected + refStore.remove(url); + } + } } } @@ -423,7 +438,7 @@ class ContextImageCache implements ImageCache { if (con != null) { if (collective) { ImageLoader im = con.getImage(url); - weakStore.put(url, im); + refStore.put(url, wrapInReference(im, url)); } con.releaseImage(url); } @@ -443,15 +458,47 @@ class ContextImageCache implements ImageCache { } } + private Reference wrapInReference(Object obj, Object key) { + return new SoftReferenceWithKey(obj, key, refQueue); + } + + private static class SoftReferenceWithKey extends SoftReference { + + private Object key; + + public SoftReferenceWithKey(Object referent, Object key, ReferenceQueue q) { + super(referent, q); + this.key = key; + } + } + public void removeContext(FOUserAgent context) { Context con = (Context) contextStore.get(context); if (con != null) { if (collective) { Map images = con.getImages(); - weakStore.putAll(images); + Iterator iter = images.entrySet().iterator(); + while (iter.hasNext()) { + Entry entry = (Entry)iter.next(); + refStore.put(entry.getKey(), + wrapInReference(entry.getValue(), entry.getKey())); + } } contextStore.remove(context); } + //House-keeping (remove cleared references) + checkReferenceQueue(); + } + + /** + * Checks the reference queue if any references have been cleared and removes them from the + * cache. + */ + private void checkReferenceQueue() { + SoftReferenceWithKey ref; + while ((ref = (SoftReferenceWithKey)refQueue.poll()) != null) { + refStore.remove(ref.key); + } } class Context { @@ -503,7 +550,7 @@ class ContextImageCache implements ImageCache { /** @see org.apache.fop.image.ImageCache#clearAll() */ public void clearAll() { - this.weakStore.clear(); + this.refStore.clear(); this.invalid.clear(); //The context-sensitive caches are not cleared so there are no negative side-effects //in a multi-threaded environment. Not that it's a good idea to use this method at diff --git a/status.xml b/status.xml index 00d1d5470..c7f3a4054 100644 --- a/status.xml +++ b/status.xml @@ -28,6 +28,15 @@ <changes> <release version="FOP Trunk"> <action context="Code" dev="JM" type="fix"> + Fixed two memory-leaks in image handling. The image cache is finally working + properly. + </action> + <action context="Code" dev="JM" type="fix" fixes-bug="39608"> + Let numeric property values without a unit be treated as pixels like in HTML. + This fixes certain NullPointerException when no units are specified. + (Note: the use of pixels in XSL-FO is discouraged!) + </action> + <action context="Code" dev="JM" type="fix"> Bugfix: Potential multi-threading issue (ConcurrentModificationException) eliminated for ElementMapping classes. </action> |