From 7ede95ce494fab902d8b23d043bc84b921280af5 Mon Sep 17 00:00:00 2001 From: Jeremias Maerki Date: Fri, 21 Jul 2006 15:01:17 +0000 Subject: [PATCH] Fixed two memory-leaks in image handling (ImageFactory and ExternalGraphic). The image cache is finally working properly. Currently implemented without the cleanup thread as done by Batik. Added ImageIO provider for handling PNG in addition to the internal codec. ImageIO proved to be faster and less memory-intensive for PNGs. ImageIO takes precedence of the internal codec. git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@424349 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/fop/fo/flow/ExternalGraphic.java | 19 ++---- .../org/apache/fop/image/ImageFactory.java | 65 ++++++++++++++++--- 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 @@ -27,6 +27,15 @@ + + Fixed two memory-leaks in image handling. The image cache is finally working + properly. + + + 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!) + Bugfix: Potential multi-threading issue (ConcurrentModificationException) eliminated for ElementMapping classes. -- 2.39.5