Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ed55c8db authored by Andy McFadden's avatar Andy McFadden
Browse files

Avoid crashing in unlockCanvasAndPost 

It's possible to update the native surface pointer while the
surface is locked (via lockCanvas).  This leads to a surprise when
the surface is unlocked.  Avoid the surprise by tracking the
locked surface separately.

Bug 10289713

Change-Id: I84346c952be859bbd91ceae7df07b91dabe0948e
parent 58514937

core/java/android/view/Surface.java

+29 −6
Original line number Diff line number Diff line
@@ -36,7 +36,7 @@ public class Surface implements Parcelable { 
            throws OutOfResourcesException;

    private static native int nativeCreateFromSurfaceControl(int surfaceControlNativeObject);



    private static native void nativeLockCanvas(int nativeObject, Canvas canvas, Rect dirty)

    private static native int nativeLockCanvas(int nativeObject, Canvas canvas, Rect dirty)

            throws OutOfResourcesException;

    private static native void nativeUnlockCanvasAndPost(int nativeObject, Canvas canvas);
@@ -72,6 +72,7 @@ public class Surface implements Parcelable { 
    final Object mLock = new Object(); // protects the native state

    private String mName;

    int mNativeObject; // package scope only for SurfaceControl access

    private int mLockedObject;

    private int mGenerationId; // incremented each time mNativeObject changes

    private final Canvas mCanvas = new CompatibleCanvas();
@@ -233,7 +234,14 @@ public class Surface implements Parcelable { 
            throws OutOfResourcesException, IllegalArgumentException {

        synchronized (mLock) {

            checkNotReleasedLocked();

            nativeLockCanvas(mNativeObject, mCanvas, inOutDirty);

            if (mLockedObject != 0) {

                // Ideally, nativeLockCanvas() would throw in this situation and prevent the

                // double-lock, but that won't happen if mNativeObject was updated.  We can't

                // abandon the old mLockedObject because it might still be in use, so instead

                // we just refuse to re-lock the Surface.

                throw new RuntimeException("Surface was already locked");

            }

            mLockedObject = nativeLockCanvas(mNativeObject, mCanvas, inOutDirty);

            return mCanvas;

        }

    }
@@ -252,7 +260,17 @@ public class Surface implements Parcelable { 


        synchronized (mLock) {

            checkNotReleasedLocked();

            nativeUnlockCanvasAndPost(mNativeObject, canvas);

            if (mNativeObject != mLockedObject) {

                Log.w(TAG, "WARNING: Surface's mNativeObject (0x" +

                        Integer.toHexString(mNativeObject) + ") != mLockedObject (0x" +

                        Integer.toHexString(mLockedObject) +")");

            }

            if (mLockedObject == 0) {

                throw new RuntimeException("Surface was not locked");

            }

            nativeUnlockCanvasAndPost(mLockedObject, canvas);

            nativeRelease(mLockedObject);

            mLockedObject = 0;

        }

    }
@@ -343,6 +361,10 @@ public class Surface implements Parcelable { 
        }



        synchronized (mLock) {

            // nativeReadFromParcel() will either return mNativeObject, or

            // create a new native Surface and return it after reducing

            // the reference count on mNativeObject.  Either way, it is

            // not necessary to call nativeRelease() here.

            mName = source.readString();

            setNativeObjectLocked(nativeReadFromParcel(mNativeObject, source));

        }
@@ -365,7 +387,8 @@ public class Surface implements Parcelable { 
    @Override

    public String toString() {

        synchronized (mLock) {

            return "Surface(name=" + mName + ")";

            return "Surface(name=" + mName + ")/@0x" +

                    Integer.toHexString(System.identityHashCode(this));

        }

    }

core/jni/android_view_Surface.cpp

+11 −4
Original line number Diff line number Diff line
@@ -196,13 +196,13 @@ static inline void swapCanvasPtr(JNIEnv* env, jobject canvasObj, SkCanvas* newCa 
  SkSafeUnref(previousCanvas);

}



static void nativeLockCanvas(JNIEnv* env, jclass clazz,

static jint nativeLockCanvas(JNIEnv* env, jclass clazz,

        jint nativeObject, jobject canvasObj, jobject dirtyRectObj) {

    sp<Surface> surface(reinterpret_cast<Surface *>(nativeObject));



    if (!isSurfaceValid(surface)) {

        doThrowIAE(env);

        return;

        return 0;

    }



    Rect dirtyRect;
@@ -223,7 +223,7 @@ static void nativeLockCanvas(JNIEnv* env, jclass clazz, 
                OutOfResourcesException :

                "java/lang/IllegalArgumentException";

        jniThrowException(env, exception, NULL);

        return;

        return 0;

    }



    // Associate a SkCanvas object to this surface
@@ -255,6 +255,13 @@ static void nativeLockCanvas(JNIEnv* env, jclass clazz, 
        env->SetIntField(dirtyRectObj, gRectClassInfo.right,  dirtyRect.right);

        env->SetIntField(dirtyRectObj, gRectClassInfo.bottom, dirtyRect.bottom);

    }



    // Create another reference to the surface and return it.  This reference

    // should be passed to nativeUnlockCanvasAndPost in place of mNativeObject,

    // because the latter could be replaced while the surface is locked.

    sp<Surface> lockedSurface(surface);

    lockedSurface->incStrong(&sRefBaseOwner);

    return (int) lockedSurface.get();

}



static void nativeUnlockCanvasAndPost(JNIEnv* env, jclass clazz,
@@ -351,7 +358,7 @@ static JNINativeMethod gSurfaceMethods[] = { 
            (void*)nativeIsValid },

    {"nativeIsConsumerRunningBehind", "(I)Z",

            (void*)nativeIsConsumerRunningBehind },

    {"nativeLockCanvas", "(ILandroid/graphics/Canvas;Landroid/graphics/Rect;)V",

    {"nativeLockCanvas", "(ILandroid/graphics/Canvas;Landroid/graphics/Rect;)I",

            (void*)nativeLockCanvas },

    {"nativeUnlockCanvasAndPost", "(ILandroid/graphics/Canvas;)V",

            (void*)nativeUnlockCanvasAndPost },