From 722487253b3049f97b5332070d685f54f57504ee Mon Sep 17 00:00:00 2001 From: Emmeran Seehuber Date: Tue, 24 Apr 2018 16:19:59 +0200 Subject: [PATCH 1/5] We must close all font files when we are finished with the document. Otherwise this may leak file handles. Those file handles should be finalized() sometime in future by the GC, but this may take way to long before all file handles are exhausted. It seems strange that you must subset a font to let its file handle close, but thats the way PDFont is implemented. Usually you always use all the fonts you load into your PDF document. But if you don't use all fonts, the unused fonts leak if they are not subsetted. We also must close all TrueTypeCollections we load. And we don't load fonts provided by file instantly, but rather use a supplier for the font. This more or less also workarounds the file handle leak, as no PDFont is loaded till it is really needed. Before this change the PDFont always has been loaded, even if it is not used. And of course was leaked if the font was not subset. --- .../pdfboxout/PdfBoxFontResolver.java | 61 ++++++++++++++++--- .../pdfboxout/PdfBoxRenderer.java | 5 +- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java index 6e25135ed..7ca6956a4 100644 --- a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java +++ b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java @@ -38,15 +38,9 @@ import org.apache.fontbox.ttf.TrueTypeCollection.TrueTypeFontProcessor; import org.apache.fontbox.ttf.TrueTypeFont; import org.apache.pdfbox.pdmodel.PDDocument; -import org.apache.pdfbox.pdmodel.font.PDFont; -import org.apache.pdfbox.pdmodel.font.PDFontDescriptor; -import org.apache.pdfbox.pdmodel.font.PDType0Font; -import org.apache.pdfbox.pdmodel.font.PDType1Font; - -import java.io.File; -import java.io.FilenameFilter; -import java.io.IOException; -import java.io.InputStream; +import org.apache.pdfbox.pdmodel.font.*; + +import java.io.*; import java.util.*; import java.util.logging.Level; @@ -59,6 +53,7 @@ public class PdfBoxFontResolver implements FontResolver { private Map _fontCache = new HashMap(); private final PDDocument _doc; private final SharedContext _sharedContext; + private List _collectionsToClose = new ArrayList(); public PdfBoxFontResolver(SharedContext sharedContext, PDDocument doc) { _sharedContext = sharedContext; @@ -70,10 +65,38 @@ public FSFont resolveFont(SharedContext renderingContext, FontSpecification spec return resolveFont(renderingContext, spec.families, spec.size, spec.fontWeight, spec.fontStyle, spec.variant); } + /** + * Free all font resources (i.e. open files), the document should already be closed. + */ + public void close() { + for (FontDescription fontDescription : _fontCache.values()) { + // If the font is not yet subset, we must subset it, otherwise we may leak a file handle + // because the PDType0Font may still have the font file open. + if (fontDescription._font != null && fontDescription._font.willBeSubset()) { + try { + fontDescription._font.subset(); + } catch (IOException e) { + e.printStackTrace(); + } + } + } + _fontCache.clear(); + + // Close all still open TrueTypeCollections + for (TrueTypeCollection collection : _collectionsToClose) { + try { + collection.close(); + } catch (IOException e) { + e.printStackTrace(); + } + } + } + @Deprecated @Override public void flushCache() { _fontFamilies = createInitialFontMap(); + close(); _fontCache = new HashMap(); } @@ -191,6 +214,7 @@ public void process(TrueTypeFont ttf) throws IOException { addFont(ttf, fontFamilyNameOverride, fontWeightOverride, fontStyleOverride, subset); } }); + _collectionsToClose.add(collection); } /** @@ -235,7 +259,24 @@ public void addFont(File fontFile, final String fontFamilyNameOverride, final In /* * We load the font using the file. */ - addFont(PDType0Font.load(_doc, fontFile), fontFamilyNameOverride, fontWeightOverride, fontStyleOverride, subset); + addFont(new FontFileSupplier(fontFile), fontFamilyNameOverride, fontWeightOverride, fontStyleOverride, subset); + } + + private static class FontFileSupplier implements FSSupplier { + private final File file; + + FontFileSupplier(File file) { + this.file = file; + } + + @Override + public InputStream supply() { + try { + return new FileInputStream(file); + } catch (FileNotFoundException e) { + return null; + } + } } diff --git a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxRenderer.java b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxRenderer.java index 8ce67ac42..85dad484f 100644 --- a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxRenderer.java +++ b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxRenderer.java @@ -834,7 +834,10 @@ public void cleanup() { _outputDevice.close(); _sharedContext.removeFromThread(); ThreadCtx.cleanup(); - + + // Close all still open font files + ((PdfBoxFontResolver)getSharedContext().getFontResolver()).close(); + if (_svgImpl != null) { try { _svgImpl.close(); From 9f8d668c54dc57fa8165cd6f5963bf232d142546 Mon Sep 17 00:00:00 2001 From: Emmeran Seehuber Date: Tue, 24 Apr 2018 16:20:30 +0200 Subject: [PATCH 2/5] Removed unused method --- .../main/java/com/openhtmltopdf/layout/SharedContext.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/SharedContext.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/SharedContext.java index ef6212d65..67329ca83 100644 --- a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/SharedContext.java +++ b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/SharedContext.java @@ -34,7 +34,6 @@ import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; -import org.w3c.dom.NodeList; import java.awt.*; import java.text.BreakIterator; @@ -167,10 +166,6 @@ public FontResolver getFontResolver() { return fontResolver; } - public void flushFonts() { - fontResolver.flushCache(); - } - /** * The media for this context */ From 3cd0bda9222798311dbe3db8d205d4472e8e4ef9 Mon Sep 17 00:00:00 2001 From: Emmeran Seehuber Date: Tue, 24 Apr 2018 16:27:04 +0200 Subject: [PATCH 3/5] Formating --- .../pdfboxout/PdfBoxFontResolver.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java index 7ca6956a4..724835fb9 100644 --- a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java +++ b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java @@ -70,15 +70,17 @@ public FSFont resolveFont(SharedContext renderingContext, FontSpecification spec */ public void close() { for (FontDescription fontDescription : _fontCache.values()) { - // If the font is not yet subset, we must subset it, otherwise we may leak a file handle - // because the PDType0Font may still have the font file open. + /* + * If the font is not yet subset, we must subset it, otherwise we may leak a + * file handle because the PDType0Font may still have the font file open. + */ if (fontDescription._font != null && fontDescription._font.willBeSubset()) { - try { - fontDescription._font.subset(); - } catch (IOException e) { - e.printStackTrace(); - } - } + try { + fontDescription._font.subset(); + } catch (IOException e) { + e.printStackTrace(); + } + } } _fontCache.clear(); From e55e602ea541f24389ffa19ea291caf052a28e55 Mon Sep 17 00:00:00 2001 From: Emmeran Seehuber Date: Tue, 24 Apr 2018 16:28:21 +0200 Subject: [PATCH 4/5] After closing all registered TTF Collections we should also clear the list. --- .../com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java index 724835fb9..9f8cbe5a3 100644 --- a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java +++ b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java @@ -65,9 +65,10 @@ public FSFont resolveFont(SharedContext renderingContext, FontSpecification spec return resolveFont(renderingContext, spec.families, spec.size, spec.fontWeight, spec.fontStyle, spec.variant); } - /** - * Free all font resources (i.e. open files), the document should already be closed. - */ + /** + * Free all font resources (i.e. open files), the document should already be + * closed. + */ public void close() { for (FontDescription fontDescription : _fontCache.values()) { /* @@ -92,6 +93,7 @@ public void close() { e.printStackTrace(); } } + _collectionsToClose.clear(); } @Deprecated From 7a4456e966b6a207c1bd98096ca4c994d4772bb1 Mon Sep 17 00:00:00 2001 From: Emmeran Seehuber Date: Tue, 24 Apr 2018 16:47:35 +0200 Subject: [PATCH 5/5] We still should use the font file when loading the font lazy. Otherwise the font file gets fully loaded into memory. --- .../pdfboxout/PdfBoxFontResolver.java | 73 ++++++++++++++----- 1 file changed, 55 insertions(+), 18 deletions(-) diff --git a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java index 9f8cbe5a3..cf5105eec 100644 --- a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java +++ b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFontResolver.java @@ -38,9 +38,15 @@ import org.apache.fontbox.ttf.TrueTypeCollection.TrueTypeFontProcessor; import org.apache.fontbox.ttf.TrueTypeFont; import org.apache.pdfbox.pdmodel.PDDocument; -import org.apache.pdfbox.pdmodel.font.*; - -import java.io.*; +import org.apache.pdfbox.pdmodel.font.PDFont; +import org.apache.pdfbox.pdmodel.font.PDFontDescriptor; +import org.apache.pdfbox.pdmodel.font.PDType0Font; +import org.apache.pdfbox.pdmodel.font.PDType1Font; + +import java.io.File; +import java.io.FilenameFilter; +import java.io.IOException; +import java.io.InputStream; import java.util.*; import java.util.logging.Level; @@ -187,10 +193,26 @@ private void addFont(TrueTypeFont trueTypeFont, String fontFamilyNameOverride, PDFont font = PDType0Font.load(_doc, trueTypeFont, subset); - addFont(font, fontFamilyNameOverride, fontWeightOverride, fontStyleOverride, subset); + addFontLazy(new PDFontSupplier(font), fontFamilyNameOverride, fontWeightOverride, fontStyleOverride, subset); } + + private static class PDFontSupplier implements FSSupplier { + private final PDFont _font; + + PDFontSupplier(PDFont font) { + _font = font; + } + + @Override + public PDFont supply() { + return _font; + } + } - private void addFont(PDFont font, String fontFamilyNameOverride, Integer fontWeightOverride, IdentValue fontStyleOverride, boolean subset) { + /** + * Add a font with a lazy loaded PDFont + */ + private void addFontLazy(FSSupplier font, String fontFamilyNameOverride, Integer fontWeightOverride, IdentValue fontStyleOverride, boolean subset) { FontFamily fontFamily = getFontFamily(fontFamilyNameOverride); FontDescription descr = new FontDescription( _doc, @@ -263,21 +285,26 @@ public void addFont(File fontFile, final String fontFamilyNameOverride, final In /* * We load the font using the file. */ - addFont(new FontFileSupplier(fontFile), fontFamilyNameOverride, fontWeightOverride, fontStyleOverride, subset); + addFontLazy(new FilePDFontSupplier(fontFile, _doc), fontFamilyNameOverride, fontWeightOverride, fontStyleOverride, subset); } - private static class FontFileSupplier implements FSSupplier { - private final File file; + /** + * Loads a Type0 font on demand + */ + private static class FilePDFontSupplier implements FSSupplier { + private final File _fontFile; + private final PDDocument _doc; - FontFileSupplier(File file) { - this.file = file; + FilePDFontSupplier(File fontFile, PDDocument doc) { + this._fontFile = fontFile; + this._doc = doc; } @Override - public InputStream supply() { + public PDFont supply() { try { - return new FileInputStream(file); - } catch (FileNotFoundException e) { + return PDType0Font.load(_doc, _fontFile); + } catch (IOException e) { return null; } } @@ -562,6 +589,7 @@ public static class FontDescription implements MinimalFontDescription { private final PDDocument _doc; private FSSupplier _supplier; + private FSSupplier _fontSupplier; private PDFont _font; private float _underlinePosition; @@ -576,10 +604,6 @@ private FontDescription(PDFont font, IdentValue style, int weight) { this(null, font, style, weight); } - public FontDescription(PDDocument doc, PDFont font) { - this(doc, font, IdentValue.NORMAL, 400); - } - private FontDescription(PDDocument doc, FSSupplier supplier, int weight, IdentValue style) { this._supplier = supplier; this._weight = weight; @@ -595,8 +619,21 @@ private FontDescription(PDDocument doc, PDFont font, IdentValue style, int weigh _doc = doc; setMetricDefaults(); } - + + private FontDescription(PDDocument doc, FSSupplier fontSupplier, IdentValue style, int weight) { + _fontSupplier = fontSupplier; + _style = style; + _weight = weight; + _supplier = null; + _doc = doc; + } + private boolean realizeFont(boolean subset) { + if (_font == null && _fontSupplier != null) { + _font = _fontSupplier.supply(); + setMetricDefaults(); + _fontSupplier = null; + } if (_font == null && _supplier != null) { InputStream is = _supplier.supply(); _supplier = null; // We only try once.