Android: Guard EGLSurface with a mutex

QAndroidPlatformOpenGlWindow creates an EGLSurface to wrap
the native Android Surface. Once this EGLSurface is returned
to QPlatformEglContext, nothing is guarding it anymore,
and Android can freely destroy the underlying Surface.
This leads to failed EGL operations when leaving from the app
or returning to it, as that is when Android destroys/recreates
the Surface.

Use the mutex from QAndroidPlatformWindow to guard setting the
Surface, and during the makeCurrent() and swapBuffers() calls.
Locking until returning from these functions ensures Android
cannot destroy the Surface in the middle of them.

Task-number: QTBUG-118231
Change-Id: I614575c42f7f0c2c17022994d3bc542726ebf039
Reviewed-by: Assam Boudjelthia <assam.boudjelthia@qt.io>
(cherry picked from commit e5513a9447771dc722ca27e553fd3e966ee7d44a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/plugins/platforms/android/qandroidplatformopenglcontext.cpp

@@ -22,19 +22,41 @@ QAndroidPlatformOpenGLContext::QAndroidPlatformOpenGLContext(const QSurfaceForma
 
 void QAndroidPlatformOpenGLContext::swapBuffers(QPlatformSurface *surface)
 {
-    if (surface->surface()->surfaceClass() == QSurface::Window &&
-            static_cast<QAndroidPlatformOpenGLWindow *>(surface)->checkNativeSurface(eglConfig())) {
-        QEGLPlatformContext::makeCurrent(surface);
+    if (surface->surface()->surfaceClass() != QSurface::Window) {
+        QEGLPlatformContext::swapBuffers(surface);
+        return;
     }
 
+    QAndroidPlatformOpenGLWindow *window = static_cast<QAndroidPlatformOpenGLWindow *>(surface);
+    // Since QEGLPlatformContext::makeCurrent() and QEGLPlatformContext::swapBuffers()
+    // will be using the eglSurface of the window, which wraps the Android Surface, we
+    // need to lock here to make sure we don't end up using a Surface already destroyed
+    // by Android
+    window->lockSurface();
+
+    if (window->checkNativeSurface(eglConfig())) {
+        // Call base class implementation directly since we are already locked
+        QEGLPlatformContext::makeCurrent(surface);
+    }
     QEGLPlatformContext::swapBuffers(surface);
+
+    window->unlockSurface();
 }
 
 bool QAndroidPlatformOpenGLContext::makeCurrent(QPlatformSurface *surface)
 {
-    return QEGLPlatformContext::makeCurrent(surface);
+    if (surface->surface()->surfaceClass() != QSurface::Window)
+        return QEGLPlatformContext::makeCurrent(surface);
+
+    QAndroidPlatformOpenGLWindow *window = static_cast<QAndroidPlatformOpenGLWindow *>(surface);
+    window->lockSurface();
+    const bool ok = QEGLPlatformContext::makeCurrent(surface);
+    window->unlockSurface();
+    return ok;
 }
 
+// Called from inside QEGLPlatformContext::swapBuffers() and QEGLPlatformContext::makeCurrent(),
+// already locked
 EGLSurface QAndroidPlatformOpenGLContext::eglSurfaceForPlatformSurface(QPlatformSurface *surface)
 {
     if (surface->surface()->surfaceClass() == QSurface::Window) {

src/plugins/platforms/android/qandroidplatformopenglwindow.cpp

@@ -50,6 +50,8 @@ void QAndroidPlatformOpenGLWindow::setGeometry(const QRect &rect)
     }
 }
 
+// Called by QAndroidPlatformOpenGLContext::eglSurfaceForPlatformSurface(),
+// surface is already locked when calling this
 EGLSurface QAndroidPlatformOpenGLWindow::eglSurface(EGLConfig config)
 {
     if (QAndroidEventDispatcherStopper::stopped() ||
@@ -57,8 +59,6 @@ EGLSurface QAndroidPlatformOpenGLWindow::eglSurface(EGLConfig config)
         return m_eglSurface;
     }
 
-    QMutexLocker lock(&m_surfaceMutex);
-
     if (!m_surfaceCreated) {
         AndroidDeadlockProtector protector;
         if (!protector.acquire())
@@ -69,25 +69,24 @@ EGLSurface QAndroidPlatformOpenGLWindow::eglSurface(EGLConfig config)
     }
 
     if (m_eglSurface == EGL_NO_SURFACE) {
-        m_surfaceMutex.unlock();
         checkNativeSurface(config);
-        m_surfaceMutex.lock();
     }
     return m_eglSurface;
 }
 
+// Only called by eglSurface() and QAndroidPlatformOpenGLContext::swapBuffers(),
+// m_surfaceMutex already locked
 bool QAndroidPlatformOpenGLWindow::checkNativeSurface(EGLConfig config)
 {
-    QMutexLocker lock(&m_surfaceMutex);
+    // Either no surface created, or the m_eglSurface already wraps the active Surface
+    // -> makeCurrent is NOT needed.
     if (!m_surfaceCreated || !m_androidSurfaceObject.isValid())
-        return false; // makeCurrent is NOT needed.
+        return false;
 
     createEgl(config);
 
-    // we've create another surface, the window should be repainted
-    QRect availableGeometry = screen()->availableGeometry();
-    if (geometry().width() > 0 && geometry().height() > 0 && availableGeometry.width() > 0 && availableGeometry.height() > 0)
-        QWindowSystemInterface::handleExposeEvent(window(), QRegion(QRect(QPoint(), geometry().size())));
+    // we've created another Surface, the window should be repainted
+    sendExpose();
     return true; // makeCurrent is needed!
 }
 

src/plugins/platforms/android/qandroidplatformwindow.cpp

@@ -304,14 +304,12 @@ void QAndroidPlatformWindow::onSurfaceChanged(QtJniTypes::Surface surface)
 {
     lockSurface();
     m_androidSurfaceObject = surface;
-    if (m_androidSurfaceObject.isValid()) // wait until we have a valid surface to draw into
-         m_surfaceWaitCondition.wakeOne();
-    unlockSurface();
-
     if (m_androidSurfaceObject.isValid()) {
-        // repaint the window, when we have a valid surface
-        sendExpose();
+        // wait until we have a valid surface to draw into
+        m_surfaceWaitCondition.wakeOne();
     }
+
+    unlockSurface();
 }
 
 void QAndroidPlatformWindow::sendExpose() const

src/plugins/platforms/android/qandroidplatformwindow.h

@@ -67,10 +67,11 @@ public:
     static bool registerNatives(QJniEnvironment &env);
     void onSurfaceChanged(QtJniTypes::Surface surface);
 
-protected:
-    void setGeometry(const QRect &rect) override;
     void lockSurface() { m_surfaceMutex.lock(); }
     void unlockSurface() { m_surfaceMutex.unlock(); }
+
+protected:
+    void setGeometry(const QRect &rect) override;
     void createSurface();
     void destroySurface();
     void sendExpose() const;
@@ -85,7 +86,11 @@ protected:
     QtJniTypes::QtWindow m_nativeQtWindow;
     SurfaceContainer m_surfaceContainerType = SurfaceContainer::SurfaceView;
     QtJniTypes::QtWindow m_nativeParentQtWindow;
-    // The Android Surface, accessed from multiple threads, guarded by m_surfaceMutex
+    // The Android Surface, accessed from multiple threads, guarded by m_surfaceMutex.
+    // If the window is using QtSurface, which is a SurfaceView subclass, this Surface will be
+    // automatically created by Android when QtSurface is in a layout and visible. If the
+    // QtSurface is detached or hidden (app goes to background), Android will automatically
+    // destroy the Surface.
     QtJniTypes::Surface m_androidSurfaceObject;
     QWaitCondition m_surfaceWaitCondition;
     bool m_surfaceCreated = false;