am ba56915a: Merge change Ie806ae6f into eclair

Merge commit 'ba56915a71cad672baff5594dbb7c9331bf01283' into eclair-plus-aosp

* commit 'ba56915a71cad672baff5594dbb7c9331bf01283':
  Fix multi-lock ordering issues in GLSurfaceView

1 files changed, 144 insertions, 121 deletions

diff --git a/opengl/java/android/opengl/GLSurfaceView.java b/opengl/java/android/opengl/GLSurfaceView.java
index 0330163..9ca57ba 100644
--- a/opengl/java/android/opengl/GLSurfaceView.java
+++ b/opengl/java/android/opengl/GLSurfaceView.java
@@ -18,7 +18,6 @@ package android.opengl;
 
 import java.io.Writer;
 import java.util.ArrayList;
-import java.util.concurrent.Semaphore;
 
 import javax.microedition.khronos.egl.EGL10;
 import javax.microedition.khronos.egl.EGL11;
@@ -943,6 +942,9 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
      * to a Renderer instance to do the actual drawing. Can be configured to
      * render continuously or on request.
      *
+     * All potentially blocking synchronization is done through the
+     * sGLThreadManager object. This avoids multiple-lock ordering issues.
+     *
      */
     class GLThread extends Thread {
         GLThread(Renderer renderer) {
@@ -962,51 +964,31 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
                 Log.i("GLThread", "starting tid=" + getId());
             }
 
-            /*
-             * When the android framework launches a second instance of
-             * an activity, the new instance's onCreate() method may be
-             * called before the first instance returns from onDestroy().
-             *
-             * This semaphore ensures that only one instance at a time
-             * accesses EGL.
-             */
             try {
                 guardedRun();
             } catch (InterruptedException e) {
                 // fall thru and exit normally
             } finally {
-                synchronized(this) {
-                    if (LOG_THREADS) {
-                        Log.i("GLThread", "exiting tid=" +  getId());
-                    }
-                    mDone = true;
-                    notifyAll();
-                }
+                sGLThreadManager.threadExiting(this);
             }
         }
 
-        private void startEgl() throws InterruptedException {
-            if (! mHaveEgl) {
-                mHaveEgl = true;
-                sGLThreadManager.start(this);
-                mEglHelper.start();
-            }
-        }
-
-        private void stopEgl() {
+        /*
+         * This private method should only be called inside a
+         * synchronized(sGLThreadManager) block.
+         */
+        private void stopEglLocked() {
             if (mHaveEgl) {
                 mHaveEgl = false;
                 mEglHelper.destroySurface();
                 mEglHelper.finish();
-                sGLThreadManager.end(this);
+                sGLThreadManager.releaseEglSurface(this);
             }
         }
 
         private void guardedRun() throws InterruptedException {
             mEglHelper = new EglHelper();
             try {
-                startEgl();
-
                 GL10 gl = null;
                 boolean tellRendererSurfaceCreated = true;
                 boolean tellRendererSurfaceChanged = true;
@@ -1015,63 +997,97 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
                  * This is our main activity thread's loop, we go until
                  * asked to quit.
                  */
-                while (!mDone) {
-
+                while (!isDone()) {
                     /*
                      *  Update the asynchronous state (window size)
                      */
-                    int w, h;
-                    boolean changed;
+                    int w = 0;
+                    int h = 0;
+                    boolean changed = false;
                     boolean needStart = false;
-                    synchronized (this) {
-                        Runnable r;
-                        while ((r = getEvent()) != null) {
-                            r.run();
-                        }
-                        if (mPaused) {
-                            stopEgl();
-                            needStart = true;
-                        }
-                        while(true) {
+                    boolean eventsWaiting = false;
+
+                    synchronized (sGLThreadManager) {
+                        while (true) {
+                            // Manage acquiring and releasing the SurfaceView
+                            // surface and the EGL surface.
+                            if (mPaused) {
+                                stopEglLocked();
+                            }
                             if (!mHasSurface) {
                                 if (!mWaitingForSurface) {
-                                    stopEgl();
+                                    stopEglLocked();
                                     mWaitingForSurface = true;
-                                    notifyAll();
+                                    sGLThreadManager.notifyAll();
                                 }
                             } else {
-                                boolean shouldHaveEgl = sGLThreadManager.shouldHaveEgl(this);
-                                if (mHaveEgl && (!shouldHaveEgl)) {
-                                    stopEgl();
-                                } else if ((!mHaveEgl) && shouldHaveEgl) {
-                                    startEgl();
-                                    needStart = true;
+                                if (!mHaveEgl) {
+                                    if (sGLThreadManager.tryAcquireEglSurface(this)) {
+                                        mHaveEgl = true;
+                                        mEglHelper.start();
+                                        mRequestRender = true;
+                                        needStart = true;
+                                    }
                                 }
                             }
-                            if (!needToWait()) {
+
+                            // Check if we need to wait. If not, update any state
+                            // that needs to be updated, copy any state that
+                            // needs to be copied, and use "break" to exit the
+                            // wait loop.
+
+                            if (mDone) {
+                                return;
+                            }
+
+                            if (mEventsWaiting) {
+                                eventsWaiting = true;
+                                mEventsWaiting = false;
+                                break;
+                            }
+
+                            if ( (! mPaused) && mHasSurface && mHaveEgl
+                                    && (mWidth > 0) && (mHeight > 0)
+                                    && (mRequestRender || (mRenderMode == RENDERMODE_CONTINUOUSLY))
+                                    ) {
+                                changed = mSizeChanged;
+                                w = mWidth;
+                                h = mHeight;
+                                mSizeChanged = false;
+                                mRequestRender = false;
+                                if (mHasSurface && mWaitingForSurface) {
+                                    changed = true;
+                                    mWaitingForSurface = false;
+                                    sGLThreadManager.notifyAll();
+                                }
                                 break;
                             }
+
+                            // By design, this is the only place where we wait().
+
                             if (LOG_THREADS) {
-                                Log.i("GLThread", "needToWait tid=" + getId());
+                                Log.i("GLThread", "waiting tid=" + getId());
                             }
-                            wait();
-                        }
-                        if (mDone) {
-                            break;
+                            sGLThreadManager.wait();
                         }
-                        changed = mSizeChanged;
-                        w = mWidth;
-                        h = mHeight;
-                        mSizeChanged = false;
-                        mRequestRender = false;
-                        if (mHasSurface && mWaitingForSurface) {
-                            changed = true;
-                            mWaitingForSurface = false;
-                            notifyAll();
+                    } // end of synchronized(sGLThreadManager)
+
+                    /*
+                     * Handle queued events
+                     */
+                    if (eventsWaiting) {
+                        Runnable r;
+                        while ((r = getEvent()) != null) {
+                            r.run();
+                            if (isDone()) {
+                                return;
+                            }
                         }
+                        // Go back and see if we need to wait to render.
+                        continue;
                     }
+
                     if (needStart) {
-                        startEgl();
                         tellRendererSurfaceCreated = true;
                         changed = true;
                     }
@@ -1102,71 +1118,63 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
                 /*
                  * clean-up everything...
                  */
-                stopEgl();
+                synchronized (sGLThreadManager) {
+                    stopEglLocked();
+                }
             }
         }
 
-        private boolean needToWait() {
-            if (mDone) {
-                return false;
+        private boolean isDone() {
+            synchronized (sGLThreadManager) {
+                return mDone;
             }
-
-            if (mPaused || (! mHasSurface) || (! mHaveEgl)) {
-                return true;
-            }
-
-            if ((mWidth > 0) && (mHeight > 0) && (mRequestRender || (mRenderMode == RENDERMODE_CONTINUOUSLY))) {
-                return false;
-            }
-
-            return true;
         }
 
         public void setRenderMode(int renderMode) {
             if ( !((RENDERMODE_WHEN_DIRTY <= renderMode) && (renderMode <= RENDERMODE_CONTINUOUSLY)) ) {
                 throw new IllegalArgumentException("renderMode");
             }
-            synchronized(this) {
+            synchronized(sGLThreadManager) {
                 mRenderMode = renderMode;
                 if (renderMode == RENDERMODE_CONTINUOUSLY) {
-                    notifyAll();
+                    sGLThreadManager.notifyAll();
                 }
             }
         }
 
         public int getRenderMode() {
-            synchronized(this) {
+            synchronized(sGLThreadManager) {
                 return mRenderMode;
             }
         }
 
         public void requestRender() {
-            synchronized(this) {
+            synchronized(sGLThreadManager) {
                 mRequestRender = true;
-                notifyAll();
+                sGLThreadManager.notifyAll();
             }
         }
 
         public void surfaceCreated() {
-            synchronized(this) {
+            synchronized(sGLThreadManager) {
                 if (LOG_THREADS) {
                     Log.i("GLThread", "surfaceCreated tid=" + getId());
                 }
                 mHasSurface = true;
-                notifyAll();
+                sGLThreadManager.notifyAll();
             }
         }
 
         public void surfaceDestroyed() {
-            synchronized(this) {
+            synchronized(sGLThreadManager) {
                 if (LOG_THREADS) {
                     Log.i("GLThread", "surfaceDestroyed tid=" + getId());
                 }
                 mHasSurface = false;
-                notifyAll();
+                sGLThreadManager.notifyAll();
                 while(!mWaitingForSurface && isAlive() && ! mDone) {
                     try {
-                        wait();
+                        sGLThreadManager.wait();
                     } catch (InterruptedException e) {
                         Thread.currentThread().interrupt();
                     }
@@ -1175,35 +1183,35 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
         }
 
         public void onPause() {
-            synchronized (this) {
+            synchronized (sGLThreadManager) {
                 mPaused = true;
-                notifyAll();
+                sGLThreadManager.notifyAll();
             }
         }
 
         public void onResume() {
-            synchronized (this) {
+            synchronized (sGLThreadManager) {
                 mPaused = false;
                 mRequestRender = true;
-                notifyAll();
+                sGLThreadManager.notifyAll();
             }
         }
 
         public void onWindowResize(int w, int h) {
-            synchronized (this) {
+            synchronized (sGLThreadManager) {
                 mWidth = w;
                 mHeight = h;
                 mSizeChanged = true;
-                notifyAll();
+                sGLThreadManager.notifyAll();
             }
         }
 
         public void requestExitAndWait() {
             // don't call this from GLThread thread or it is a guaranteed
             // deadlock!
-            synchronized(this) {
+            synchronized(sGLThreadManager) {
                 mDone = true;
-                notifyAll();
+                sGLThreadManager.notifyAll();
             }
             try {
                 join();
@@ -1219,6 +1227,10 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
         public void queueEvent(Runnable r) {
             synchronized(this) {
                 mEventQueue.add(r);
+                synchronized(sGLThreadManager) {
+                    mEventsWaiting = true;
+                    sGLThreadManager.notifyAll();
+                }
             }
         }
 
@@ -1232,6 +1244,8 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
             return null;
         }
 
+        // Once the thread is started, all accesses to the following member
+        // variables are protected by the sGLThreadManager monitor
         private boolean mDone;
         private boolean mPaused;
         private boolean mHasSurface;
@@ -1241,6 +1255,9 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
         private int mHeight;
         private int mRenderMode;
         private boolean mRequestRender;
+        private boolean mEventsWaiting;
+        // End of member variables protected by the sGLThreadManager monitor.
+
         private Renderer mRenderer;
         private ArrayList<Runnable> mEventQueue = new ArrayList<Runnable>();
         private EglHelper mEglHelper;
@@ -1286,37 +1303,43 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
         }
     }
 
-    static class GLThreadManager {
-        public boolean shouldHaveEgl(GLThread thread) {
-            synchronized(this) {
-                return thread == mMostRecentGLThread || mMostRecentGLThread == null;
+    private static class GLThreadManager {
+
+        public synchronized void threadExiting(GLThread thread) {
+            if (LOG_THREADS) {
+                Log.i("GLThread", "exiting tid=" +  thread.getId());
             }
-        }
-        public void start(GLThread thread) throws InterruptedException {
-            GLThread oldThread = null;
-            synchronized(this) {
-                oldThread = mMostRecentGLThread;
-                mMostRecentGLThread = thread;
+            thread.mDone = true;
+            if (mEglOwner == thread) {
+                mEglOwner = null;
             }
-            if (oldThread != null) {
-                synchronized(oldThread) {
-                    oldThread.notifyAll();
-                }
+            notifyAll();
+        }
+
+        /*
+         * Tries once to acquire the right to use an EGL
+         * surface. Does not block.
+         * @return true if the right to use an EGL surface was acquired.
+         */
+        public synchronized boolean tryAcquireEglSurface(GLThread thread) {
+            if (mEglOwner == thread || mEglOwner == null) {
+                mEglOwner = thread;
+                notifyAll();
+                return true;
             }
-            sEglSemaphore.acquire();
+            return false;
         }
-        public void end(GLThread thread) {
-            sEglSemaphore.release();
-            synchronized(this) {
-                if (mMostRecentGLThread == thread) {
-                    mMostRecentGLThread = null;
-                }
+
+        public synchronized void releaseEglSurface(GLThread thread) {
+            if (mEglOwner == thread) {
+                mEglOwner = null;
             }
+            notifyAll();
         }
-        private GLThread mMostRecentGLThread;
+
+        private GLThread mEglOwner;
     }
 
-    private static final Semaphore sEglSemaphore = new Semaphore(1);
     private static final GLThreadManager sGLThreadManager = new GLThreadManager();
     private boolean mSizeChanged = true;