Greetings,
The freeNativeResources function in freetypeScaler.c frees a pointer
obtained from FreeType internals:
stream = scalerInfo->face->stream;
FT_Done_Face(scalerInfo->face);
...snip...
if (stream != NULL) {
free(stream);
}
This is done in order to free a FT_StreamRec that Java allocates in some
cases when initialising the native scaler. However, the current approach
is wrong because:
- FT_Done_Face will itself also free the face->stream pointer, if
that memory was allocated by FreeType itself instead of by Java.
- FT_FaceRec.stream is explicitly noted to be a private field in
FreeType documentation.
With the way Java uses FreeType, FT ends up allocating the FT_StreamRec
structure in Java's Type 1 font case. As a result we've been observing
JVM crashes due to a double free when:
- a java.awt.Font is constructed from Type 1 font data
- it becomes garbage
- the associated FreetypeFontScaler is disposed
The attached FontDisposeTest.java can demonstrate the crash. Pass the
path to a Type 1 font file as argument and it will load the font and
force native scaler disposal. If memory allocation debugging is not
active by default, it can be forced on by setting an environment
variable 'MALLOC_CHECK_=3' (glibc, for Windows see PageHeap).
A Type 1 font file can be found from Ghostscript
(ftp://ftp.gnu.org/gnu/ghostscript/gnu-gs-fonts-other-6.0.tar.gz)
FreeType records whether the pointer in FT_FaceRec.stream was allocated
externally or by itself in the FT_FaceRec.face_flags field. However the
documentation for FT_FACE_FLAG_EXTERNAL_STREAM says
Used internally by FreeType to indicate that a face's stream was provided by
the client application and should not be destroyed when FT_Done_Face is called.
Don't read or test this flag.
Therefore Java should maintain it's own copy of the stream pointer so it
can know with certainty what memory it needs to free, if any.
Attached are patches to fix the bug in jdk8u and jdk9. I've added a
field `faceStream` to the FTScalerInfo struct to keep track of the
Java-allocated stream.
PS. if this is not the correct mailing list, or a valid way of
submitting a patch, could you point me to the correct one?
-- Heikki Aitakangas
import java.awt.Font;
import java.awt.Graphics2D;
import java.awt.font.FontRenderContext;
import java.awt.image.BufferedImage;
import java.io.FileInputStream;
import java.io.InputStream;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
public class FontDisposeTest
{
public static void main(String[] args) throws Throwable
{
// The bug only happens with Type 1 fonts. The Ghostscript font files
// should be commonly available. From distro pacakge or
// ftp://ftp.gnu.org/gnu/ghostscript/gnu-gs-fonts-other-6.0.tar.gz
String path = args[0];
// Load
System.out.printf("Loading font from '%s'... ",path);
InputStream stream = new FileInputStream(path);
Font font = Font.createFont(Font.TYPE1_FONT,stream);
System.out.printf(" done.\n");
// Ensure native bits have been generated
BufferedImage img = new BufferedImage(100,100,BufferedImage.TYPE_INT_ARGB);
Graphics2D g2d = img.createGraphics();
FontRenderContext frc = g2d.getFontRenderContext();
font.getLineMetrics("derp",frc);
System.out.printf("Font native scaler initialised.\n");
// Force disposal - I have not yet figured out a way to get it to happen
// naturally in a test case context. Just waiting and/or calling
// System.gc() is not sufficient.
Field font2DHandleField = Font.class.getDeclaredField("font2DHandle");
font2DHandleField.setAccessible(true);
sun.font.Font2DHandle font2DHandle =
(sun.font.Font2DHandle)font2DHandleField.get(font);
sun.font.Font2D font2D = font2DHandle.font2D;
sun.font.Type1Font type1Font = (sun.font.Type1Font)font2D;
Method getScalerMethod =
sun.font.Type1Font.class.getDeclaredMethod("getScaler");
getScalerMethod.setAccessible(true);
sun.font.FontScaler scaler =
(sun.font.FontScaler)getScalerMethod.invoke(type1Font);
System.out.println("Forcing font native scaler disposal.\n");
scaler.dispose();
System.out.println("\nIf nothing happened, try again with environment variable 'MALLOC_CHECK_=3'.\n");
}
}
# HG changeset patch
# User Heikki Aitakangas <heikki.aitakan...@documill.com>
# Date 1438693351 -10800
# Tue Aug 04 16:02:31 2015 +0300
# Node ID d35700befb98097f7457e066844e662edd1825f6
# Parent be5faa9c77042f202106c18f4e8ea211137b4a3b
Fixed a double free bug in FreeType native scaler.
diff --git a/src/share/native/sun/font/freetypeScaler.c b/src/share/native/sun/font/freetypeScaler.c
--- a/src/share/native/sun/font/freetypeScaler.c
+++ b/src/share/native/sun/font/freetypeScaler.c
@@ -60,6 +60,7 @@
JNIEnv* env;
FT_Library library;
FT_Face face;
+ FT_Stream faceStream;
jobject font2D;
jobject directBuffer;
@@ -107,15 +108,10 @@
if (scalerInfo == NULL)
return;
- //apparently Done_Face will only close the stream
- // but will not relase the memory of stream structure.
- // We need to free it explicitly to avoid leak.
- //Direct access to the stream field might be not ideal solution as
- // it is considred to be "private".
- //Alternatively we could have stored pointer to the structure
- // in the scalerInfo but this will increase size of the structure
- // for no good reason
- stream = scalerInfo->face->stream;
+ // FT_Done_Face always closes the stream, but only frees the memory
+ // of the data structure if it was internally allocated by FT.
+ // We hold on to a pointer to the stream structure if we provide it
+ // ourselves, so that we can free it here.
FT_Done_Face(scalerInfo->face);
FT_Done_FreeType(scalerInfo->library);
@@ -128,8 +124,8 @@
free(scalerInfo->fontData);
}
- if (stream != NULL) {
- free(stream);
+ if (scalerInfo->faceStream != NULL) {
+ free(scalerInfo->faceStream);
}
free(scalerInfo);
@@ -302,6 +298,10 @@
&ft_open_args,
indexInCollection,
&scalerInfo->face);
+
+ if (!error) {
+ scalerInfo->faceStream = ftstream;
+ }
}
if (error || scalerInfo->directBuffer == NULL) {
free(ftstream);
# HG changeset patch
# User Heikki Aitakangas <heikki.aitakan...@documill.com>
# Date 1438697194 -10800
# Tue Aug 04 17:06:34 2015 +0300
# Node ID 95c9d9e47bd44bba1f52eee9d91b972dfa632441
# Parent ab2f946581d1f030c1324ebd5f5c1fb56447039b
Fixed a double free bug in FreeType native scaler.
diff --git a/src/java.desktop/share/native/libfontmanager/freetypeScaler.c b/src/java.desktop/share/native/libfontmanager/freetypeScaler.c
--- a/src/java.desktop/share/native/libfontmanager/freetypeScaler.c
+++ b/src/java.desktop/share/native/libfontmanager/freetypeScaler.c
@@ -60,6 +60,7 @@
JNIEnv* env;
FT_Library library;
FT_Face face;
+ FT_Stream faceStream;
jobject font2D;
jobject directBuffer;
@@ -107,15 +108,10 @@
if (scalerInfo == NULL)
return;
- //apparently Done_Face will only close the stream
- // but will not relase the memory of stream structure.
- // We need to free it explicitly to avoid leak.
- //Direct access to the stream field might be not ideal solution as
- // it is considred to be "private".
- //Alternatively we could have stored pointer to the structure
- // in the scalerInfo but this will increase size of the structure
- // for no good reason
- stream = scalerInfo->face->stream;
+ // FT_Done_Face always closes the stream, but only frees the memory
+ // of the data structure if it was internally allocated by FT.
+ // We hold on to a pointer to the stream structure if we provide it
+ // ourselves, so that we can free it here.
FT_Done_Face(scalerInfo->face);
FT_Done_FreeType(scalerInfo->library);
@@ -128,8 +124,8 @@
free(scalerInfo->fontData);
}
- if (stream != NULL) {
- free(stream);
+ if (scalerInfo->faceStream != NULL) {
+ free(scalerInfo->faceStream);
}
free(scalerInfo);
@@ -302,6 +298,10 @@
&ft_open_args,
indexInCollection,
&scalerInfo->face);
+
+ if (!error) {
+ scalerInfo->faceStream = ftstream;
+ }
}
if (error || scalerInfo->directBuffer == NULL) {
free(ftstream);
