Merge "Really make Surface thread-safe." into jb-mr2-dev
diff --git a/core/java/android/view/Surface.java b/core/java/android/view/Surface.java
index 4989c3a..e0786f7 100644
--- a/core/java/android/view/Surface.java
+++ b/core/java/android/view/Surface.java
@@ -34,19 +34,21 @@
private static native int nativeCreateFromSurfaceTexture(SurfaceTexture surfaceTexture)
throws OutOfResourcesException;
+ private static native int nativeCreateFromSurfaceControl(int surfaceControlNativeObject);
- private native Canvas nativeLockCanvas(int nativeObject, Rect dirty);
- private native void nativeUnlockCanvasAndPost(int nativeObject, Canvas canvas);
+ private static native void nativeLockCanvas(int nativeObject, Canvas canvas, Rect dirty)
+ throws OutOfResourcesException;
+ private static native void nativeUnlockCanvasAndPost(int nativeObject, Canvas canvas);
private static native void nativeRelease(int nativeObject);
private static native boolean nativeIsValid(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 void nativeWriteToParcel(int nativeObject, Parcel dest);
public static final Parcelable.Creator<Surface> CREATOR =
new Parcelable.Creator<Surface>() {
+ @Override
public Surface createFromParcel(Parcel source) {
try {
Surface s = new Surface();
@@ -57,26 +59,20 @@
return null;
}
}
+
+ @Override
public Surface[] newArray(int size) {
return new Surface[size];
}
};
private final CloseGuard mCloseGuard = CloseGuard.get();
+
+ // Guarded state.
+ final Object mLock = new Object(); // protects the native state
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
-
- // protects the native state
- private final Object mNativeObjectLock = new Object();
-
- private int mGenerationId; // incremented each time mNativeSurface changes
- @SuppressWarnings("UnusedDeclaration")
+ private int mGenerationId; // incremented each time mNativeObject changes
private final Canvas mCanvas = new CompatibleCanvas();
// A matrix to scale the matrix set by application. This is set to null for
@@ -125,21 +121,22 @@
throw new IllegalArgumentException("surfaceTexture must not be null");
}
- mName = surfaceTexture.toString();
- try {
- mNativeObject = nativeCreateFromSurfaceTexture(surfaceTexture);
- } catch (OutOfResourcesException ex) {
- // We can't throw OutOfResourcesException because it would be an API change.
- throw new RuntimeException(ex);
+ synchronized (mLock) {
+ mName = surfaceTexture.toString();
+ try {
+ setNativeObjectLocked(nativeCreateFromSurfaceTexture(surfaceTexture));
+ } catch (OutOfResourcesException 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() */
private Surface(int nativeObject) {
- mNativeObject = nativeObject;
- mCloseGuard.open("release");
+ synchronized (mLock) {
+ setNativeObjectLocked(nativeObject);
+ }
}
@Override
@@ -160,13 +157,11 @@
* This will make the surface invalid.
*/
public void release() {
- synchronized (mNativeObjectLock) {
+ synchronized (mLock) {
if (mNativeObject != 0) {
nativeRelease(mNativeObject);
- mNativeObject = 0;
- mGenerationId++;
+ setNativeObjectLocked(0);
}
- mCloseGuard.close();
}
}
@@ -187,7 +182,7 @@
* Otherwise returns false.
*/
public boolean isValid() {
- synchronized (mNativeObjectLock) {
+ synchronized (mLock) {
if (mNativeObject == 0) return false;
return nativeIsValid(mNativeObject);
}
@@ -201,7 +196,9 @@
* @hide
*/
public int getGenerationId() {
- return mGenerationId;
+ synchronized (mLock) {
+ return mGenerationId;
+ }
}
/**
@@ -211,7 +208,7 @@
* @hide
*/
public boolean isConsumerRunningBehind() {
- synchronized (mNativeObjectLock) {
+ synchronized (mLock) {
checkNotReleasedLocked();
return nativeIsConsumerRunningBehind(mNativeObject);
}
@@ -234,9 +231,10 @@
*/
public Canvas lockCanvas(Rect inOutDirty)
throws OutOfResourcesException, IllegalArgumentException {
- synchronized (mNativeObjectLock) {
+ synchronized (mLock) {
checkNotReleasedLocked();
- return nativeLockCanvas(mNativeObject, inOutDirty);
+ nativeLockCanvas(mNativeObject, mCanvas, inOutDirty);
+ return mCanvas;
}
}
@@ -247,7 +245,12 @@
* @param canvas The canvas previously obtained from {@link #lockCanvas}.
*/
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();
nativeUnlockCanvasAndPost(mNativeObject, canvas);
}
@@ -273,7 +276,6 @@
}
}
-
/**
* 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.
@@ -287,22 +289,24 @@
if (other == null) {
throw new IllegalArgumentException("other must not be null");
}
- if (other.mNativeObject == 0) {
+
+ int surfaceControlPtr = other.mNativeObject;
+ if (surfaceControlPtr == 0) {
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
- mCloseGuard.close();
+ int newNativeObject = nativeCreateFromSurfaceControl(surfaceControlPtr);
+
+ synchronized (mLock) {
+ if (mNativeObject != 0) {
+ 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
* @hide
* @deprecated
@@ -313,21 +317,18 @@
throw new IllegalArgumentException("other must not be null");
}
if (other != this) {
- synchronized (mNativeObjectLock) {
+ final int newPtr;
+ synchronized (other.mLock) {
+ newPtr = other.mNativeObject;
+ other.setNativeObjectLocked(0);
+ }
+
+ synchronized (mLock) {
if (mNativeObject != 0) {
- // release our reference to our native object
nativeRelease(mNativeObject);
}
- // transfer the reference from other to us
- if (other.mNativeObject != 0 && mNativeObject == 0) {
- mCloseGuard.open("release");
- }
- mNativeObject = other.mNativeObject;
- mGenerationId++;
+ setNativeObjectLocked(newPtr);
}
- other.mNativeObject = 0;
- other.mGenerationId++;
- other.mCloseGuard.close();
}
}
@@ -340,14 +341,10 @@
if (source == null) {
throw new IllegalArgumentException("source must not be null");
}
- synchronized (mNativeObjectLock) {
+
+ synchronized (mLock) {
mName = source.readString();
- int nativeObject = nativeReadFromParcel(mNativeObject, source);
- if (nativeObject !=0 && mNativeObject == 0) {
- mCloseGuard.open("release");
- }
- mNativeObject = nativeObject;
- mGenerationId++;
+ setNativeObjectLocked(nativeReadFromParcel(mNativeObject, source));
}
}
@@ -356,7 +353,7 @@
if (dest == null) {
throw new IllegalArgumentException("dest must not be null");
}
- synchronized (mNativeObjectLock) {
+ synchronized (mLock) {
dest.writeString(mName);
nativeWriteToParcel(mNativeObject, dest);
}
@@ -367,7 +364,27 @@
@Override
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 @@
mOrigMatrix.set(m);
}
}
-
- private void checkNotReleasedLocked() {
- if (mNativeObject == 0) throw new NullPointerException(
- "mNativeObject is null. Have you called release() already?");
- }
}
diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java
index e869d09..6b530ef 100644
--- a/core/java/android/view/SurfaceControl.java
+++ b/core/java/android/view/SurfaceControl.java
@@ -496,8 +496,14 @@
if (displayToken == 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) {
diff --git a/core/jni/android_view_Surface.cpp b/core/jni/android_view_Surface.cpp
index a41a389..9a19ce5 100644
--- a/core/jni/android_view_Surface.cpp
+++ b/core/jni/android_view_Surface.cpp
@@ -55,8 +55,7 @@
static struct {
jclass clazz;
jfieldID mNativeObject;
- jfieldID mNativeObjectLock;
- jfieldID mCanvas;
+ jfieldID mLock;
jmethodID ctor;
} gSurfaceClassInfo;
@@ -93,7 +92,7 @@
sp<Surface> android_view_Surface_getSurface(JNIEnv* env, jobject surfaceObj) {
sp<Surface> sur;
jobject lock = env->GetObjectField(surfaceObj,
- gSurfaceClassInfo.mNativeObjectLock);
+ gSurfaceClassInfo.mLock);
if (env->MonitorEnter(lock) == JNI_OK) {
sur = reinterpret_cast<Surface *>(
env->GetIntField(surfaceObj, gSurfaceClassInfo.mNativeObject));
@@ -200,12 +199,13 @@
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));
if (!isSurfaceValid(surface)) {
doThrowIAE(env);
- return NULL;
+ return;
}
// get dirty region
@@ -232,11 +232,10 @@
OutOfResourcesException :
"java/lang/IllegalArgumentException";
jniThrowException(env, exception, NULL);
- return NULL;
+ return;
}
// Associate a SkCanvas object to this surface
- jobject canvasObj = env->GetObjectField(surfaceObj, gSurfaceClassInfo.mCanvas);
env->SetIntField(canvasObj, gCanvasClassInfo.mSurfaceFormat, outBuffer.format);
SkBitmap bitmap;
@@ -277,17 +276,10 @@
env->SetIntField(dirtyRectObj, gRectClassInfo.right, bounds.right);
env->SetIntField(dirtyRectObj, gRectClassInfo.bottom, bounds.bottom);
}
-
- return canvasObj;
}
-static void nativeUnlockCanvasAndPost(JNIEnv* env, jobject surfaceObj, jint nativeObject, jobject canvasObj) {
- jobject ownCanvasObj = env->GetObjectField(surfaceObj, gSurfaceClassInfo.mCanvas);
- if (!env->IsSameObject(ownCanvasObj, canvasObj)) {
- doThrowIAE(env);
- return;
- }
-
+static void nativeUnlockCanvasAndPost(JNIEnv* env, jclass clazz,
+ jint nativeObject, jobject canvasObj) {
sp<Surface> surface(reinterpret_cast<Surface *>(nativeObject));
if (!isSurfaceValid(surface)) {
return;
@@ -306,8 +298,8 @@
// ----------------------------------------------------------------------------
-static jint nativeCopyFrom(JNIEnv* env, jclass clazz,
- jint nativeObject, jint surfaceControlNativeObj) {
+static jint nativeCreateFromSurfaceControl(JNIEnv* env, jclass clazz,
+ jint surfaceControlNativeObj) {
/*
* This is used by the WindowManagerService just after constructing
* a Surface and is necessary for returning the Surface reference to
@@ -315,17 +307,11 @@
*/
sp<SurfaceControl> ctrl(reinterpret_cast<SurfaceControl *>(surfaceControlNativeObj));
- sp<Surface> other(ctrl->getSurface());
- if (other != NULL) {
- other->incStrong(&sRefBaseOwner);
+ sp<Surface> surface(ctrl->getSurface());
+ if (surface != NULL) {
+ surface->incStrong(&sRefBaseOwner);
}
-
- sp<Surface> sur(reinterpret_cast<Surface *>(nativeObject));
- if (sur != NULL) {
- sur->decStrong(&sRefBaseOwner);
- }
-
- return int(other.get());
+ return reinterpret_cast<jint>(surface.get());
}
static jint nativeReadFromParcel(JNIEnv* env, jclass clazz,
@@ -386,12 +372,12 @@
(void*)nativeIsValid },
{"nativeIsConsumerRunningBehind", "(I)Z",
(void*)nativeIsConsumerRunningBehind },
- {"nativeLockCanvas", "(ILandroid/graphics/Rect;)Landroid/graphics/Canvas;",
+ {"nativeLockCanvas", "(ILandroid/graphics/Canvas;Landroid/graphics/Rect;)V",
(void*)nativeLockCanvas },
{"nativeUnlockCanvasAndPost", "(ILandroid/graphics/Canvas;)V",
(void*)nativeUnlockCanvasAndPost },
- {"nativeCopyFrom", "(II)I",
- (void*)nativeCopyFrom },
+ {"nativeCreateFromSurfaceControl", "(I)I",
+ (void*)nativeCreateFromSurfaceControl },
{"nativeReadFromParcel", "(ILandroid/os/Parcel;)I",
(void*)nativeReadFromParcel },
{"nativeWriteToParcel", "(ILandroid/os/Parcel;)V",
@@ -407,10 +393,8 @@
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.mLock =
+ env->GetFieldID(gSurfaceClassInfo.clazz, "mLock", "Ljava/lang/Object;");
gSurfaceClassInfo.ctor = env->GetMethodID(gSurfaceClassInfo.clazz, "<init>", "(I)V");
clazz = env->FindClass("android/graphics/Canvas");