Bug 1416015 - Ensure SurfaceTexture desctruction happens correctly r=jnicol draft
authorJames Willcox <snorp@snorp.net>
Wed, 15 Nov 2017 15:20:26 -0600
changeset 700912 75cbb877465693b1cf281ae8a57ab45c0fa89e82
parent 700742 8db0afdca2336b1afc364dcaf7678bd877080183
child 741020 a17be88959b45d751182a182927f6cb950262be8
push id89992
push userbmo:snorp@snorp.net
push dateMon, 20 Nov 2017 23:35:23 +0000
reviewersjnicol
bugs1416015
milestone59.0a1
Bug 1416015 - Ensure SurfaceTexture desctruction happens correctly r=jnicol MozReview-Commit-ID: I4X1jQQC7ry
gfx/layers/opengl/CompositorOGL.cpp
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/GeckoSurfaceTexture.java
widget/android/GeneratedJNIWrappers.cpp
widget/android/GeneratedJNIWrappers.h
--- a/gfx/layers/opengl/CompositorOGL.cpp
+++ b/gfx/layers/opengl/CompositorOGL.cpp
@@ -42,16 +42,17 @@
 #include "nsString.h"                   // for nsString, nsAutoCString, etc
 #include "ScopedGLHelpers.h"
 #include "GLReadTexImageHelper.h"
 #include "GLBlitTextureImageHelper.h"
 #include "HeapCopyOfStackArray.h"
 
 #if MOZ_WIDGET_ANDROID
 #include "TexturePoolOGL.h"
+#include "GeneratedJNIWrappers.h"
 #endif
 
 #include "GeckoProfiler.h"
 
 namespace mozilla {
 
 using namespace std;
 using namespace gfx;
@@ -669,16 +670,17 @@ CompositorOGL::BeginFrame(const nsIntReg
     MakeCurrent();
   }
 
   mPixelsPerFrame = width * height;
   mPixelsFilled = 0;
 
 #ifdef MOZ_WIDGET_ANDROID
   TexturePoolOGL::Fill(gl());
+  java::GeckoSurfaceTexture::DestroyUnused((int64_t)mGLContext.get());
 #endif
 
   // Default blend function implements "OVER"
   mGLContext->fBlendFuncSeparate(LOCAL_GL_ONE, LOCAL_GL_ONE_MINUS_SRC_ALPHA,
                                  LOCAL_GL_ONE, LOCAL_GL_ONE_MINUS_SRC_ALPHA);
   mGLContext->fEnable(LOCAL_GL_BLEND);
 
   RefPtr<CompositingRenderTargetOGL> rt =
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/GeckoSurfaceTexture.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/GeckoSurfaceTexture.java
@@ -1,48 +1,71 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.gfx;
 
 import android.graphics.SurfaceTexture;
+import android.os.Handler;
 import android.util.Log;
 
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.HashMap;
+import java.util.LinkedList;
 
 import org.mozilla.gecko.annotation.WrapForJNI;
 import org.mozilla.gecko.AppConstants.Versions;
 
 public final class GeckoSurfaceTexture extends SurfaceTexture {
     private static final String LOGTAG = "GeckoSurfaceTexture";
     private static volatile int sNextHandle = 1;
     private static HashMap<Integer, GeckoSurfaceTexture> sSurfaceTextures = new HashMap<Integer, GeckoSurfaceTexture>();
 
+
+    private static HashMap<Long, LinkedList<GeckoSurfaceTexture>> sUnusedTextures =
+        new HashMap<Long, LinkedList<GeckoSurfaceTexture>>();
+
     private int mHandle;
     private boolean mIsSingleBuffer;
 
     private long mAttachedContext;
     private int mTexName;
 
     private GeckoSurfaceTexture.Callbacks mListener;
     private AtomicInteger mUseCount;
+    private boolean mFinalized;
 
     private GeckoSurfaceTexture(int handle) {
         super(0);
         init(handle, false);
     }
 
     private GeckoSurfaceTexture(int handle, boolean singleBufferMode) {
         super(0, singleBufferMode);
         init(handle, singleBufferMode);
     }
 
+    @Override
+    public void finalize() {
+        // We only want finalize() to be called once
+        if (mFinalized) {
+            return;
+        }
+
+        mFinalized = true;
+
+        try {
+            super.finalize();
+        } catch (Throwable t) {
+            Log.e(LOGTAG, "Failed to finalize SurfaceTexture", t);
+        }
+    }
+
     private void init(int handle, boolean singleBufferMode) {
         mHandle = handle;
         mIsSingleBuffer = singleBufferMode;
         mUseCount = new AtomicInteger(1);
 
         // Start off detached
         detachFromGLContext();
     }
@@ -53,37 +76,37 @@ public final class GeckoSurfaceTexture e
     }
 
     @WrapForJNI
     public int getTexName() {
         return mTexName;
     }
 
     @WrapForJNI(exceptionMode = "nsresult")
-    public void attachToGLContext(long context, int texName) {
+    public synchronized void attachToGLContext(long context, int texName) {
         if (context == mAttachedContext && texName == mTexName) {
             return;
         }
 
         attachToGLContext(texName);
 
         mAttachedContext = context;
         mTexName = texName;
     }
 
     @Override
     @WrapForJNI(exceptionMode = "nsresult")
-    public void detachFromGLContext() {
+    public synchronized void detachFromGLContext() {
         super.detachFromGLContext();
 
         mAttachedContext = mTexName = 0;
     }
 
     @WrapForJNI
-    public boolean isAttachedToGLContext(long context) {
+    public synchronized boolean isAttachedToGLContext(long context) {
         return mAttachedContext == context;
     }
 
     @WrapForJNI
     public boolean isSingleBuffer() {
         return mIsSingleBuffer;
     }
 
@@ -122,41 +145,73 @@ public final class GeckoSurfaceTexture e
     }
 
     @WrapForJNI
     public static boolean isSingleBufferSupported() {
         return Versions.feature19Plus;
     }
 
     @WrapForJNI
-    public void incrementUse() {
+    public synchronized void incrementUse() {
         mUseCount.incrementAndGet();
     }
 
     @WrapForJNI
-    public void decrementUse() {
+    public synchronized void decrementUse() {
         int useCount = mUseCount.decrementAndGet();
 
         if (useCount == 0) {
+            setListener(null);
+
             synchronized (sSurfaceTextures) {
                 sSurfaceTextures.remove(mHandle);
             }
 
-            setListener(null);
-
-            if (Versions.feature16Plus) {
-                try {
-                    detachFromGLContext();
-                } catch (Exception e) {
-                    // This can throw if the EGL context is not current
-                    // but we can't do anything about that now.
-                }
+            if (mAttachedContext == 0) {
+                release();
+                return;
             }
 
-            release();
+            synchronized (sUnusedTextures) {
+                LinkedList<GeckoSurfaceTexture> list = sUnusedTextures.get(mAttachedContext);
+                if (list == null) {
+                    list = new LinkedList<GeckoSurfaceTexture>();
+                    sUnusedTextures.put(mAttachedContext, list);
+                }
+                list.addFirst(this);
+            }
+        }
+    }
+
+    @WrapForJNI
+    public static void destroyUnused(long context) {
+        LinkedList<GeckoSurfaceTexture> list;
+        synchronized(sUnusedTextures) {
+            list = sUnusedTextures.remove(context);
+        }
+
+        if (list == null) {
+            return;
+        }
+
+        for (GeckoSurfaceTexture tex : list) {
+            try {
+                if (tex.isSingleBuffer()) {
+                   tex.releaseTexImage();
+                }
+
+                tex.detachFromGLContext();
+                tex.release();
+
+                // We need to manually call finalize here, otherwise we can run out
+                // of file descriptors if the GC doesn't kick in soon enough. Bug 1416015.
+                tex.finalize();
+            } catch (Exception e) {
+                Log.e(LOGTAG, "Failed to destroy SurfaceTexture", e);
+            }
         }
     }
 
     public static GeckoSurfaceTexture acquire(boolean singleBufferMode) {
         if (singleBufferMode && !isSingleBufferSupported()) {
             throw new IllegalArgumentException("single buffer mode not supported on API version < 19");
         }
 
--- a/widget/android/GeneratedJNIWrappers.cpp
+++ b/widget/android/GeneratedJNIWrappers.cpp
@@ -1209,16 +1209,24 @@ auto GeckoSurfaceTexture::AttachToGLCont
 constexpr char GeckoSurfaceTexture::DecrementUse_t::name[];
 constexpr char GeckoSurfaceTexture::DecrementUse_t::signature[];
 
 auto GeckoSurfaceTexture::DecrementUse() const -> void
 {
     return mozilla::jni::Method<DecrementUse_t>::Call(GeckoSurfaceTexture::mCtx, nullptr);
 }
 
+constexpr char GeckoSurfaceTexture::DestroyUnused_t::name[];
+constexpr char GeckoSurfaceTexture::DestroyUnused_t::signature[];
+
+auto GeckoSurfaceTexture::DestroyUnused(int64_t a0) -> void
+{
+    return mozilla::jni::Method<DestroyUnused_t>::Call(GeckoSurfaceTexture::Context(), nullptr, a0);
+}
+
 constexpr char GeckoSurfaceTexture::DetachFromGLContext_t::name[];
 constexpr char GeckoSurfaceTexture::DetachFromGLContext_t::signature[];
 
 auto GeckoSurfaceTexture::DetachFromGLContext() const -> nsresult
 {
     nsresult rv = NS_OK;
     mozilla::jni::Method<DetachFromGLContext_t>::Call(GeckoSurfaceTexture::mCtx, &rv);
     return rv;
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -3708,16 +3708,36 @@ public:
         static const mozilla::jni::CallingThread callingThread =
                 mozilla::jni::CallingThread::ANY;
         static const mozilla::jni::DispatchTarget dispatchTarget =
                 mozilla::jni::DispatchTarget::CURRENT;
     };
 
     auto DecrementUse() const -> void;
 
+    struct DestroyUnused_t {
+        typedef GeckoSurfaceTexture Owner;
+        typedef void ReturnType;
+        typedef void SetterType;
+        typedef mozilla::jni::Args<
+                int64_t> Args;
+        static constexpr char name[] = "destroyUnused";
+        static constexpr char signature[] =
+                "(J)V";
+        static const bool isStatic = true;
+        static const mozilla::jni::ExceptionMode exceptionMode =
+                mozilla::jni::ExceptionMode::ABORT;
+        static const mozilla::jni::CallingThread callingThread =
+                mozilla::jni::CallingThread::ANY;
+        static const mozilla::jni::DispatchTarget dispatchTarget =
+                mozilla::jni::DispatchTarget::CURRENT;
+    };
+
+    static auto DestroyUnused(int64_t) -> void;
+
     struct DetachFromGLContext_t {
         typedef GeckoSurfaceTexture Owner;
         typedef void ReturnType;
         typedef void SetterType;
         typedef mozilla::jni::Args<> Args;
         static constexpr char name[] = "detachFromGLContext";
         static constexpr char signature[] =
                 "()V";