Reduce risk of memory corruption due to finalization.

Many classes in graphics/java and elsewhere deallocate native memory
in a finalizer on the assumption that instance methods can no longer
be called once the finalizer has been called.  This is incorrect if
the object can be used, possibly indirectly, from another finalizer,
possibly one in the application.

This is the initial installment of a patch to cause such post-finalization
uses to at least see a null pointer rather than causing memory corruption
by accessing deallocated native memory. This should make it possible to
identify and fix such finalization ordering issues.

There are more graphics classes that need this treatment, and probably
many more in other subsystems.

This solution is < 100% effective if finalizers can be invoked
concurrently.  We currently promise that they aren't.

(In my opinion, the real cause here is a language spec bug.  But that ship
has sailed.)

Bug: 18178237
Change-Id: I844cf1e0fbb190407389c4f8e8f072752cca6198
This commit is contained in:
Hans Boehm
2014-11-11 17:01:37 -08:00
parent 596fd38af2
commit ffa84e008c
13 changed files with 20 additions and 7 deletions

View File

@@ -147,11 +147,12 @@ public class Interpolator {
@Override
protected void finalize() throws Throwable {
nativeDestructor(native_instance);
native_instance = 0; // Other finalizers can still call us.
}
private int mValueCount;
private int mFrameCount;
private final long native_instance;
private long native_instance;
private static native long nativeConstructor(int valueCount, int frameCount);
private static native void nativeDestructor(long native_instance);

View File

@@ -25,6 +25,7 @@ public class MaskFilter {
protected void finalize() throws Throwable {
nativeDestructor(native_instance);
native_instance = 0; // Other finalizers can still call us.
}
private static native void nativeDestructor(long native_filter);

View File

@@ -827,6 +827,7 @@ public class Matrix {
protected void finalize() throws Throwable {
try {
finalizer(native_instance);
native_instance = 0; // Other finalizers can still call us.
} finally {
super.finalize();
}

View File

@@ -71,7 +71,7 @@ public class NinePatch {
*
* @hide
*/
public final long mNativeChunk;
public long mNativeChunk;
private Paint mPaint;
private String mSrcName;
@@ -121,6 +121,7 @@ public class NinePatch {
if (mNativeChunk != 0) {
// only attempt to destroy correctly initilized chunks
nativeFinalize(mNativeChunk);
mNativeChunk = 0;
}
} finally {
super.finalize();

View File

@@ -2219,6 +2219,7 @@ public class Paint {
protected void finalize() throws Throwable {
try {
finalizer(mNativePaint);
mNativePaint = 0;
} finally {
super.finalize();
}

View File

@@ -27,7 +27,7 @@ public class Path {
/**
* @hide
*/
public final long mNativePath;
public long mNativePath;
/**
* @hide
@@ -746,6 +746,7 @@ public class Path {
protected void finalize() throws Throwable {
try {
finalizer(mNativePath);
mNativePath = 0; // Other finalizers can still call us.
} finally {
super.finalize();
}

View File

@@ -25,6 +25,7 @@ public class PathEffect {
protected void finalize() throws Throwable {
nativeDestructor(native_instance);
native_instance = 0; // Other finalizers can still call us.
}
private static native void nativeDestructor(long native_patheffect);

View File

@@ -142,6 +142,7 @@ public class PathMeasure {
protected void finalize() throws Throwable {
native_destroy(native_instance);
native_instance = 0; // Other finalizers can still call us.
}
private static native long native_create(long native_path, boolean forceClosed);
@@ -154,6 +155,6 @@ public class PathMeasure {
private static native boolean native_nextContour(long native_instance);
private static native void native_destroy(long native_instance);
/* package */private final long native_instance;
/* package */private long native_instance;
}

View File

@@ -29,7 +29,7 @@ import java.io.OutputStream;
*/
public class Picture {
private Canvas mRecordingCanvas;
private final long mNativePicture;
private long mNativePicture;
private static final int WORKING_STREAM_STORAGE = 16 * 1024;
@@ -60,6 +60,7 @@ public class Picture {
protected void finalize() throws Throwable {
try {
nativeDestructor(mNativePicture);
mNativePicture = 0;
} finally {
super.finalize();
}

View File

@@ -30,7 +30,7 @@ public class Region implements Parcelable {
/**
* @hide
*/
public final long mNativeRegion;
public long mNativeRegion;
// the native values for these must match up with the enum in SkRegion.h
public enum Op {
@@ -380,6 +380,7 @@ public class Region implements Parcelable {
protected void finalize() throws Throwable {
try {
nativeDestructor(mNativeRegion);
mNativeRegion = 0;
} finally {
super.finalize();
}

View File

@@ -43,12 +43,13 @@ public class RegionIterator {
protected void finalize() throws Throwable {
nativeDestructor(mNativeIter);
mNativeIter = 0; // Other finalizers can still call us.
}
private static native long nativeConstructor(long native_region);
private static native void nativeDestructor(long native_iter);
private static native boolean nativeNext(long native_iter, Rect r);
private final long mNativeIter;
private long mNativeIter;
}

View File

@@ -90,6 +90,7 @@ public class Shader {
super.finalize();
} finally {
nativeDestructor(native_instance);
native_instance = 0; // Other finalizers can still call us.
}
}

View File

@@ -358,6 +358,7 @@ public class Typeface {
protected void finalize() throws Throwable {
try {
nativeUnref(native_instance);
native_instance = 0; // Other finalizers can still call us.
} finally {
super.finalize();
}