Merge "Really make Surface thread-safe." into jb-mr2-dev

This commit is contained in:
Jeff Brown
2013-05-03 02:11:02 +00:00
committed by Android (Google) Code Review
3 changed files with 108 additions and 106 deletions

View File

@ -34,19 +34,21 @@ public class Surface implements Parcelable {
private static native int nativeCreateFromSurfaceTexture(SurfaceTexture surfaceTexture) private static native int nativeCreateFromSurfaceTexture(SurfaceTexture surfaceTexture)
throws OutOfResourcesException; throws OutOfResourcesException;
private static native int nativeCreateFromSurfaceControl(int surfaceControlNativeObject);
private native Canvas nativeLockCanvas(int nativeObject, Rect dirty); private static native void nativeLockCanvas(int nativeObject, Canvas canvas, Rect dirty)
private native void nativeUnlockCanvasAndPost(int nativeObject, Canvas canvas); throws OutOfResourcesException;
private static native void nativeUnlockCanvasAndPost(int nativeObject, Canvas canvas);
private static native void nativeRelease(int nativeObject); private static native void nativeRelease(int nativeObject);
private static native boolean nativeIsValid(int nativeObject); private static native boolean nativeIsValid(int nativeObject);
private static native boolean nativeIsConsumerRunningBehind(int nativeObject); private static native boolean nativeIsConsumerRunningBehind(int nativeObject);
private static native int nativeCopyFrom(int nativeObject, int surfaceControlNativeObject);
private static native int nativeReadFromParcel(int nativeObject, Parcel source); private static native int nativeReadFromParcel(int nativeObject, Parcel source);
private static native void nativeWriteToParcel(int nativeObject, Parcel dest); private static native void nativeWriteToParcel(int nativeObject, Parcel dest);
public static final Parcelable.Creator<Surface> CREATOR = public static final Parcelable.Creator<Surface> CREATOR =
new Parcelable.Creator<Surface>() { new Parcelable.Creator<Surface>() {
@Override
public Surface createFromParcel(Parcel source) { public Surface createFromParcel(Parcel source) {
try { try {
Surface s = new Surface(); Surface s = new Surface();
@ -57,26 +59,20 @@ public class Surface implements Parcelable {
return null; return null;
} }
} }
@Override
public Surface[] newArray(int size) { public Surface[] newArray(int size) {
return new Surface[size]; return new Surface[size];
} }
}; };
private final CloseGuard mCloseGuard = CloseGuard.get(); private final CloseGuard mCloseGuard = CloseGuard.get();
// Guarded state.
final Object mLock = new Object(); // protects the native state
private String mName; private String mName;
// Note: These fields are accessed by native code.
// The mSurfaceControl will only be present for Surfaces used by the window
// server or system processes. When this class is parceled we defer to the
// mSurfaceControl to do the parceling. Otherwise we parcel the
// mNativeSurface.
int mNativeObject; // package scope only for SurfaceControl access int mNativeObject; // package scope only for SurfaceControl access
private int mGenerationId; // incremented each time mNativeObject changes
// 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(); private final Canvas mCanvas = new CompatibleCanvas();
// A matrix to scale the matrix set by application. This is set to null for // A matrix to scale the matrix set by application. This is set to null for
@ -125,21 +121,22 @@ public class Surface implements Parcelable {
throw new IllegalArgumentException("surfaceTexture must not be null"); throw new IllegalArgumentException("surfaceTexture must not be null");
} }
mName = surfaceTexture.toString(); synchronized (mLock) {
try { mName = surfaceTexture.toString();
mNativeObject = nativeCreateFromSurfaceTexture(surfaceTexture); try {
} catch (OutOfResourcesException ex) { setNativeObjectLocked(nativeCreateFromSurfaceTexture(surfaceTexture));
// We can't throw OutOfResourcesException because it would be an API change. } catch (OutOfResourcesException ex) {
throw new RuntimeException(ex); // We can't throw OutOfResourcesException because it would be an API change.
throw new RuntimeException(ex);
}
} }
mCloseGuard.open("release");
} }
/* called from android_view_Surface_createFromIGraphicBufferProducer() */ /* called from android_view_Surface_createFromIGraphicBufferProducer() */
private Surface(int nativeObject) { private Surface(int nativeObject) {
mNativeObject = nativeObject; synchronized (mLock) {
mCloseGuard.open("release"); setNativeObjectLocked(nativeObject);
}
} }
@Override @Override
@ -160,13 +157,11 @@ public class Surface implements Parcelable {
* This will make the surface invalid. * This will make the surface invalid.
*/ */
public void release() { public void release() {
synchronized (mNativeObjectLock) { synchronized (mLock) {
if (mNativeObject != 0) { if (mNativeObject != 0) {
nativeRelease(mNativeObject); nativeRelease(mNativeObject);
mNativeObject = 0; setNativeObjectLocked(0);
mGenerationId++;
} }
mCloseGuard.close();
} }
} }
@ -187,7 +182,7 @@ public class Surface implements Parcelable {
* Otherwise returns false. * Otherwise returns false.
*/ */
public boolean isValid() { public boolean isValid() {
synchronized (mNativeObjectLock) { synchronized (mLock) {
if (mNativeObject == 0) return false; if (mNativeObject == 0) return false;
return nativeIsValid(mNativeObject); return nativeIsValid(mNativeObject);
} }
@ -201,7 +196,9 @@ public class Surface implements Parcelable {
* @hide * @hide
*/ */
public int getGenerationId() { public int getGenerationId() {
return mGenerationId; synchronized (mLock) {
return mGenerationId;
}
} }
/** /**
@ -211,7 +208,7 @@ public class Surface implements Parcelable {
* @hide * @hide
*/ */
public boolean isConsumerRunningBehind() { public boolean isConsumerRunningBehind() {
synchronized (mNativeObjectLock) { synchronized (mLock) {
checkNotReleasedLocked(); checkNotReleasedLocked();
return nativeIsConsumerRunningBehind(mNativeObject); return nativeIsConsumerRunningBehind(mNativeObject);
} }
@ -234,9 +231,10 @@ public class Surface implements Parcelable {
*/ */
public Canvas lockCanvas(Rect inOutDirty) public Canvas lockCanvas(Rect inOutDirty)
throws OutOfResourcesException, IllegalArgumentException { throws OutOfResourcesException, IllegalArgumentException {
synchronized (mNativeObjectLock) { synchronized (mLock) {
checkNotReleasedLocked(); checkNotReleasedLocked();
return nativeLockCanvas(mNativeObject, inOutDirty); nativeLockCanvas(mNativeObject, mCanvas, inOutDirty);
return mCanvas;
} }
} }
@ -247,7 +245,12 @@ public class Surface implements Parcelable {
* @param canvas The canvas previously obtained from {@link #lockCanvas}. * @param canvas The canvas previously obtained from {@link #lockCanvas}.
*/ */
public void unlockCanvasAndPost(Canvas canvas) { public void unlockCanvasAndPost(Canvas canvas) {
synchronized (mNativeObjectLock) { if (canvas != mCanvas) {
throw new IllegalArgumentException("canvas object must be the same instance that "
+ "was previously returned by lockCanvas");
}
synchronized (mLock) {
checkNotReleasedLocked(); checkNotReleasedLocked();
nativeUnlockCanvasAndPost(mNativeObject, canvas); nativeUnlockCanvasAndPost(mNativeObject, canvas);
} }
@ -273,7 +276,6 @@ public class Surface implements Parcelable {
} }
} }
/** /**
* Copy another surface to this one. This surface now holds a reference * Copy another surface to this one. This surface now holds a reference
* to the same data as the original surface, and is -not- the owner. * to the same data as the original surface, and is -not- the owner.
@ -287,22 +289,24 @@ public class Surface implements Parcelable {
if (other == null) { if (other == null) {
throw new IllegalArgumentException("other must not be null"); throw new IllegalArgumentException("other must not be null");
} }
if (other.mNativeObject == 0) {
int surfaceControlPtr = other.mNativeObject;
if (surfaceControlPtr == 0) {
throw new NullPointerException( throw new NullPointerException(
"SurfaceControl native object is null. Are you using a released SurfaceControl?"); "SurfaceControl native object is null. Are you using a released SurfaceControl?");
} }
synchronized (mNativeObjectLock) { int newNativeObject = nativeCreateFromSurfaceControl(surfaceControlPtr);
mNativeObject = nativeCopyFrom(mNativeObject, other.mNativeObject);
if (mNativeObject == 0) { synchronized (mLock) {
// nativeCopyFrom released our reference if (mNativeObject != 0) {
mCloseGuard.close(); nativeRelease(mNativeObject);
} }
mGenerationId++; setNativeObjectLocked(newNativeObject);
} }
} }
/** /**
* This is intended to be used by {@link SurfaceView.updateWindow} only. * This is intended to be used by {@link SurfaceView#updateWindow} only.
* @param other access is not thread safe * @param other access is not thread safe
* @hide * @hide
* @deprecated * @deprecated
@ -313,21 +317,18 @@ public class Surface implements Parcelable {
throw new IllegalArgumentException("other must not be null"); throw new IllegalArgumentException("other must not be null");
} }
if (other != this) { if (other != this) {
synchronized (mNativeObjectLock) { final int newPtr;
synchronized (other.mLock) {
newPtr = other.mNativeObject;
other.setNativeObjectLocked(0);
}
synchronized (mLock) {
if (mNativeObject != 0) { if (mNativeObject != 0) {
// release our reference to our native object
nativeRelease(mNativeObject); nativeRelease(mNativeObject);
} }
// transfer the reference from other to us setNativeObjectLocked(newPtr);
if (other.mNativeObject != 0 && mNativeObject == 0) {
mCloseGuard.open("release");
}
mNativeObject = other.mNativeObject;
mGenerationId++;
} }
other.mNativeObject = 0;
other.mGenerationId++;
other.mCloseGuard.close();
} }
} }
@ -340,14 +341,10 @@ public class Surface implements Parcelable {
if (source == null) { if (source == null) {
throw new IllegalArgumentException("source must not be null"); throw new IllegalArgumentException("source must not be null");
} }
synchronized (mNativeObjectLock) {
synchronized (mLock) {
mName = source.readString(); mName = source.readString();
int nativeObject = nativeReadFromParcel(mNativeObject, source); setNativeObjectLocked(nativeReadFromParcel(mNativeObject, source));
if (nativeObject !=0 && mNativeObject == 0) {
mCloseGuard.open("release");
}
mNativeObject = nativeObject;
mGenerationId++;
} }
} }
@ -356,7 +353,7 @@ public class Surface implements Parcelable {
if (dest == null) { if (dest == null) {
throw new IllegalArgumentException("dest must not be null"); throw new IllegalArgumentException("dest must not be null");
} }
synchronized (mNativeObjectLock) { synchronized (mLock) {
dest.writeString(mName); dest.writeString(mName);
nativeWriteToParcel(mNativeObject, dest); nativeWriteToParcel(mNativeObject, dest);
} }
@ -367,7 +364,27 @@ public class Surface implements Parcelable {
@Override @Override
public String toString() { public String toString() {
return "Surface(name=" + mName + ")"; synchronized (mLock) {
return "Surface(name=" + mName + ")";
}
}
private void setNativeObjectLocked(int ptr) {
if (mNativeObject != ptr) {
if (mNativeObject == 0 && ptr != 0) {
mCloseGuard.open("release");
} else if (mNativeObject != 0 && ptr == 0) {
mCloseGuard.close();
}
mNativeObject = ptr;
mGenerationId += 1;
}
}
private void checkNotReleasedLocked() {
if (mNativeObject == 0) {
throw new IllegalStateException("Surface has already been released.");
}
} }
/** /**
@ -451,9 +468,4 @@ public class Surface implements Parcelable {
mOrigMatrix.set(m); mOrigMatrix.set(m);
} }
} }
private void checkNotReleasedLocked() {
if (mNativeObject == 0) throw new NullPointerException(
"mNativeObject is null. Have you called release() already?");
}
} }

View File

@ -496,8 +496,14 @@ public class SurfaceControl {
if (displayToken == null) { if (displayToken == null) {
throw new IllegalArgumentException("displayToken must not be null"); throw new IllegalArgumentException("displayToken must not be null");
} }
int nativeSurface = surface != null ? surface.mNativeObject : 0;
nativeSetDisplaySurface(displayToken, nativeSurface); if (surface != null) {
synchronized (surface.mLock) {
nativeSetDisplaySurface(displayToken, surface.mNativeObject);
}
} else {
nativeSetDisplaySurface(displayToken, 0);
}
} }
public static IBinder createDisplay(String name, boolean secure) { public static IBinder createDisplay(String name, boolean secure) {

View File

@ -55,8 +55,7 @@ static const char* const OutOfResourcesException =
static struct { static struct {
jclass clazz; jclass clazz;
jfieldID mNativeObject; jfieldID mNativeObject;
jfieldID mNativeObjectLock; jfieldID mLock;
jfieldID mCanvas;
jmethodID ctor; jmethodID ctor;
} gSurfaceClassInfo; } gSurfaceClassInfo;
@ -93,7 +92,7 @@ sp<ANativeWindow> android_view_Surface_getNativeWindow(JNIEnv* env, jobject surf
sp<Surface> android_view_Surface_getSurface(JNIEnv* env, jobject surfaceObj) { sp<Surface> android_view_Surface_getSurface(JNIEnv* env, jobject surfaceObj) {
sp<Surface> sur; sp<Surface> sur;
jobject lock = env->GetObjectField(surfaceObj, jobject lock = env->GetObjectField(surfaceObj,
gSurfaceClassInfo.mNativeObjectLock); gSurfaceClassInfo.mLock);
if (env->MonitorEnter(lock) == JNI_OK) { if (env->MonitorEnter(lock) == JNI_OK) {
sur = reinterpret_cast<Surface *>( sur = reinterpret_cast<Surface *>(
env->GetIntField(surfaceObj, gSurfaceClassInfo.mNativeObject)); env->GetIntField(surfaceObj, gSurfaceClassInfo.mNativeObject));
@ -200,12 +199,13 @@ static inline void swapCanvasPtr(JNIEnv* env, jobject canvasObj, SkCanvas* newCa
SkSafeUnref(previousCanvas); SkSafeUnref(previousCanvas);
} }
static jobject nativeLockCanvas(JNIEnv* env, jobject surfaceObj, jint nativeObject, jobject dirtyRectObj) { static void nativeLockCanvas(JNIEnv* env, jclass clazz,
jint nativeObject, jobject canvasObj, jobject dirtyRectObj) {
sp<Surface> surface(reinterpret_cast<Surface *>(nativeObject)); sp<Surface> surface(reinterpret_cast<Surface *>(nativeObject));
if (!isSurfaceValid(surface)) { if (!isSurfaceValid(surface)) {
doThrowIAE(env); doThrowIAE(env);
return NULL; return;
} }
// get dirty region // get dirty region
@ -232,11 +232,10 @@ static jobject nativeLockCanvas(JNIEnv* env, jobject surfaceObj, jint nativeObje
OutOfResourcesException : OutOfResourcesException :
"java/lang/IllegalArgumentException"; "java/lang/IllegalArgumentException";
jniThrowException(env, exception, NULL); jniThrowException(env, exception, NULL);
return NULL; return;
} }
// Associate a SkCanvas object to this surface // Associate a SkCanvas object to this surface
jobject canvasObj = env->GetObjectField(surfaceObj, gSurfaceClassInfo.mCanvas);
env->SetIntField(canvasObj, gCanvasClassInfo.mSurfaceFormat, outBuffer.format); env->SetIntField(canvasObj, gCanvasClassInfo.mSurfaceFormat, outBuffer.format);
SkBitmap bitmap; SkBitmap bitmap;
@ -277,17 +276,10 @@ static jobject nativeLockCanvas(JNIEnv* env, jobject surfaceObj, jint nativeObje
env->SetIntField(dirtyRectObj, gRectClassInfo.right, bounds.right); env->SetIntField(dirtyRectObj, gRectClassInfo.right, bounds.right);
env->SetIntField(dirtyRectObj, gRectClassInfo.bottom, bounds.bottom); env->SetIntField(dirtyRectObj, gRectClassInfo.bottom, bounds.bottom);
} }
return canvasObj;
} }
static void nativeUnlockCanvasAndPost(JNIEnv* env, jobject surfaceObj, jint nativeObject, jobject canvasObj) { static void nativeUnlockCanvasAndPost(JNIEnv* env, jclass clazz,
jobject ownCanvasObj = env->GetObjectField(surfaceObj, gSurfaceClassInfo.mCanvas); jint nativeObject, jobject canvasObj) {
if (!env->IsSameObject(ownCanvasObj, canvasObj)) {
doThrowIAE(env);
return;
}
sp<Surface> surface(reinterpret_cast<Surface *>(nativeObject)); sp<Surface> surface(reinterpret_cast<Surface *>(nativeObject));
if (!isSurfaceValid(surface)) { if (!isSurfaceValid(surface)) {
return; return;
@ -306,8 +298,8 @@ static void nativeUnlockCanvasAndPost(JNIEnv* env, jobject surfaceObj, jint nati
// ---------------------------------------------------------------------------- // ----------------------------------------------------------------------------
static jint nativeCopyFrom(JNIEnv* env, jclass clazz, static jint nativeCreateFromSurfaceControl(JNIEnv* env, jclass clazz,
jint nativeObject, jint surfaceControlNativeObj) { jint surfaceControlNativeObj) {
/* /*
* This is used by the WindowManagerService just after constructing * This is used by the WindowManagerService just after constructing
* a Surface and is necessary for returning the Surface reference to * a Surface and is necessary for returning the Surface reference to
@ -315,17 +307,11 @@ static jint nativeCopyFrom(JNIEnv* env, jclass clazz,
*/ */
sp<SurfaceControl> ctrl(reinterpret_cast<SurfaceControl *>(surfaceControlNativeObj)); sp<SurfaceControl> ctrl(reinterpret_cast<SurfaceControl *>(surfaceControlNativeObj));
sp<Surface> other(ctrl->getSurface()); sp<Surface> surface(ctrl->getSurface());
if (other != NULL) { if (surface != NULL) {
other->incStrong(&sRefBaseOwner); surface->incStrong(&sRefBaseOwner);
} }
return reinterpret_cast<jint>(surface.get());
sp<Surface> sur(reinterpret_cast<Surface *>(nativeObject));
if (sur != NULL) {
sur->decStrong(&sRefBaseOwner);
}
return int(other.get());
} }
static jint nativeReadFromParcel(JNIEnv* env, jclass clazz, static jint nativeReadFromParcel(JNIEnv* env, jclass clazz,
@ -386,12 +372,12 @@ static JNINativeMethod gSurfaceMethods[] = {
(void*)nativeIsValid }, (void*)nativeIsValid },
{"nativeIsConsumerRunningBehind", "(I)Z", {"nativeIsConsumerRunningBehind", "(I)Z",
(void*)nativeIsConsumerRunningBehind }, (void*)nativeIsConsumerRunningBehind },
{"nativeLockCanvas", "(ILandroid/graphics/Rect;)Landroid/graphics/Canvas;", {"nativeLockCanvas", "(ILandroid/graphics/Canvas;Landroid/graphics/Rect;)V",
(void*)nativeLockCanvas }, (void*)nativeLockCanvas },
{"nativeUnlockCanvasAndPost", "(ILandroid/graphics/Canvas;)V", {"nativeUnlockCanvasAndPost", "(ILandroid/graphics/Canvas;)V",
(void*)nativeUnlockCanvasAndPost }, (void*)nativeUnlockCanvasAndPost },
{"nativeCopyFrom", "(II)I", {"nativeCreateFromSurfaceControl", "(I)I",
(void*)nativeCopyFrom }, (void*)nativeCreateFromSurfaceControl },
{"nativeReadFromParcel", "(ILandroid/os/Parcel;)I", {"nativeReadFromParcel", "(ILandroid/os/Parcel;)I",
(void*)nativeReadFromParcel }, (void*)nativeReadFromParcel },
{"nativeWriteToParcel", "(ILandroid/os/Parcel;)V", {"nativeWriteToParcel", "(ILandroid/os/Parcel;)V",
@ -407,10 +393,8 @@ int register_android_view_Surface(JNIEnv* env)
gSurfaceClassInfo.clazz = jclass(env->NewGlobalRef(clazz)); gSurfaceClassInfo.clazz = jclass(env->NewGlobalRef(clazz));
gSurfaceClassInfo.mNativeObject = gSurfaceClassInfo.mNativeObject =
env->GetFieldID(gSurfaceClassInfo.clazz, "mNativeObject", "I"); env->GetFieldID(gSurfaceClassInfo.clazz, "mNativeObject", "I");
gSurfaceClassInfo.mNativeObjectLock = gSurfaceClassInfo.mLock =
env->GetFieldID(gSurfaceClassInfo.clazz, "mNativeObjectLock", "Ljava/lang/Object;"); env->GetFieldID(gSurfaceClassInfo.clazz, "mLock", "Ljava/lang/Object;");
gSurfaceClassInfo.mCanvas =
env->GetFieldID(gSurfaceClassInfo.clazz, "mCanvas", "Landroid/graphics/Canvas;");
gSurfaceClassInfo.ctor = env->GetMethodID(gSurfaceClassInfo.clazz, "<init>", "(I)V"); gSurfaceClassInfo.ctor = env->GetMethodID(gSurfaceClassInfo.clazz, "<init>", "(I)V");
clazz = env->FindClass("android/graphics/Canvas"); clazz = env->FindClass("android/graphics/Canvas");