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

Commit 7c116b54 authored by Mathias Agopian's avatar Mathias Agopian
Browse files

make Surface.java internal state thread-safe

it's still incorrect to use Surface from different
threads, however this shouldn't result to native crashes
anymore.

Bug: 8328715
Change-Id: I89ac5cc1218dc5aa0e35f8e6d4737879a442f0c6
parent 2e6ce4f7
Loading
Loading
Loading
Loading
+62 −43
Original line number Diff line number Diff line
@@ -72,6 +72,9 @@ public class Surface implements Parcelable {
    // mNativeSurface.
    int mNativeObject; // package scope only for SurfaceControl access

    // protects the native state
    private final Object mNativeObjectLock = new Object();

    private int mGenerationId; // incremented each time mNativeSurface changes
    @SuppressWarnings("UnusedDeclaration")
    private final Canvas mCanvas = new CompatibleCanvas();
@@ -157,6 +160,7 @@ public class Surface implements Parcelable {
     * This will make the surface invalid.
     */
    public void release() {
        synchronized (mNativeObjectLock) {
            if (mNativeObject != 0) {
                nativeRelease(mNativeObject);
                mNativeObject = 0;
@@ -164,6 +168,7 @@ public class Surface implements Parcelable {
            }
            mCloseGuard.close();
        }
    }

    /**
     * Free all server-side state associated with this surface and
@@ -182,9 +187,11 @@ public class Surface implements Parcelable {
     * Otherwise returns false.
     */
    public boolean isValid() {
        synchronized (mNativeObjectLock) {
            if (mNativeObject == 0) return false;
            return nativeIsValid(mNativeObject);
        }
    }

    /**
     * Gets the generation number of this surface, incremented each time
@@ -204,9 +211,11 @@ public class Surface implements Parcelable {
     * @hide
     */
    public boolean isConsumerRunningBehind() {
        checkNotReleased();
        synchronized (mNativeObjectLock) {
            checkNotReleasedLocked();
            return nativeIsConsumerRunningBehind(mNativeObject);
        }
    }

    /**
     * Gets a {@link Canvas} for drawing into this surface.
@@ -225,9 +234,11 @@ public class Surface implements Parcelable {
     */
    public Canvas lockCanvas(Rect inOutDirty)
            throws OutOfResourcesException, IllegalArgumentException {
        checkNotReleased();
        synchronized (mNativeObjectLock) {
            checkNotReleasedLocked();
            return nativeLockCanvas(mNativeObject, inOutDirty);
        }
    }

    /**
     * Posts the new contents of the {@link Canvas} to the surface and
@@ -236,9 +247,11 @@ public class Surface implements Parcelable {
     * @param canvas The canvas previously obtained from {@link #lockCanvas}.
     */
    public void unlockCanvasAndPost(Canvas canvas) {
        checkNotReleased();
        synchronized (mNativeObjectLock) {
            checkNotReleasedLocked();
            nativeUnlockCanvasAndPost(mNativeObject, canvas);
        }
    }

    /** 
     * @deprecated This API has been removed and is not supported.  Do not use.
@@ -278,6 +291,7 @@ public class Surface implements Parcelable {
            throw new NullPointerException(
                    "SurfaceControl native object is null. Are you using a released SurfaceControl?");
        }
        synchronized (mNativeObjectLock) {
            mNativeObject = nativeCopyFrom(mNativeObject, other.mNativeObject);
            if (mNativeObject == 0) {
                // nativeCopyFrom released our reference
@@ -285,20 +299,21 @@ public class Surface implements Parcelable {
            }
            mGenerationId++;
        }
    }

    /**
     * Transfer the native state from 'other' to this surface, releasing it
     * from 'other'.  This is for use in the client side for drawing into a
     * surface; not guaranteed to work on the window manager side.
     * This is for use by the client to move the underlying surface from
     * one Surface object to another, in particular in SurfaceFlinger.
     * @hide.
     * This is intended to be used by {@link SurfaceView.updateWindow} only.
     * @param other access is not thread safe
     * @hide
     * @deprecated
     */
    @Deprecated
    public void transferFrom(Surface other) {
        if (other == null) {
            throw new IllegalArgumentException("other must not be null");
        }
        if (other != this) {
            synchronized (mNativeObjectLock) {
                if (mNativeObject != 0) {
                    // release our reference to our native object
                    nativeRelease(mNativeObject);
@@ -309,7 +324,7 @@ public class Surface implements Parcelable {
                }
                mNativeObject = other.mNativeObject;
                mGenerationId++;

            }
            other.mNativeObject = 0;
            other.mGenerationId++;
            other.mCloseGuard.close();
@@ -325,6 +340,7 @@ public class Surface implements Parcelable {
        if (source == null) {
            throw new IllegalArgumentException("source must not be null");
        }
        synchronized (mNativeObjectLock) {
            mName = source.readString();
            int nativeObject = nativeReadFromParcel(mNativeObject, source);
            if (nativeObject !=0 && mNativeObject == 0) {
@@ -333,14 +349,17 @@ public class Surface implements Parcelable {
            mNativeObject = nativeObject;
            mGenerationId++;
        }
    }

    @Override
    public void writeToParcel(Parcel dest, int flags) {
        if (dest == null) {
            throw new IllegalArgumentException("dest must not be null");
        }
        synchronized (mNativeObjectLock) {
            dest.writeString(mName);
            nativeWriteToParcel(mNativeObject, dest);
        }
        if ((flags & Parcelable.PARCELABLE_WRITE_RETURN_VALUE) != 0) {
            release();
        }
@@ -433,7 +452,7 @@ public class Surface implements Parcelable {
        }
    }

    private void checkNotReleased() {
    private void checkNotReleasedLocked() {
        if (mNativeObject == 0) throw new NullPointerException(
                "mNativeObject is null. Have you called release() already?");
    }
+12 −2
Original line number Diff line number Diff line
@@ -55,6 +55,7 @@ static const char* const OutOfResourcesException =
static struct {
    jclass clazz;
    jfieldID mNativeObject;
    jfieldID mNativeObjectLock;
    jfieldID mCanvas;
    jmethodID ctor;
} gSurfaceClassInfo;
@@ -90,8 +91,15 @@ sp<ANativeWindow> android_view_Surface_getNativeWindow(JNIEnv* env, jobject surf
}

sp<Surface> android_view_Surface_getSurface(JNIEnv* env, jobject surfaceObj) {
    return reinterpret_cast<Surface *>(
    sp<Surface> sur;
    jobject lock = env->GetObjectField(surfaceObj,
            gSurfaceClassInfo.mNativeObjectLock);
    if (env->MonitorEnter(lock) == JNI_OK) {
        sur = reinterpret_cast<Surface *>(
                env->GetIntField(surfaceObj, gSurfaceClassInfo.mNativeObject));
        env->MonitorExit(lock);
    }
    return sur;
}

jobject android_view_Surface_createFromIGraphicBufferProducer(JNIEnv* env,
@@ -399,6 +407,8 @@ int register_android_view_Surface(JNIEnv* env)
    gSurfaceClassInfo.clazz = jclass(env->NewGlobalRef(clazz));
    gSurfaceClassInfo.mNativeObject =
            env->GetFieldID(gSurfaceClassInfo.clazz, "mNativeObject", "I");
    gSurfaceClassInfo.mNativeObjectLock =
            env->GetFieldID(gSurfaceClassInfo.clazz, "mNativeObjectLock", "Ljava/lang/Object;");
    gSurfaceClassInfo.mCanvas =
            env->GetFieldID(gSurfaceClassInfo.clazz, "mCanvas", "Landroid/graphics/Canvas;");
    gSurfaceClassInfo.ctor = env->GetMethodID(gSurfaceClassInfo.clazz, "<init>", "(I)V");