don’t leak the ExternalContext object

FEngine doesn’t take ownership of ExternalContext if
it’s passed as a parameter, however, in the common
case it’s not, and FEngine owns it — and must take
care of destroying it.


Also fix this behavior in “single thread” mode,
the given ExternalContext was ignored.

filament/include/filament/driver/ExternalContext.h

@@ -24,6 +24,9 @@
 #include <utils/compiler.h>
 
 namespace filament {
+namespace details {
+class FEngine;
+}
 
 class Driver;
 
@@ -38,11 +41,6 @@ public:
         uintptr_t image;
     };
 
-    // Creates the platform-specific ExternalContext object. The caller takes ownership and is
-    // responsible for destroying it. Initialization of the backend API is deferred until
-    // createDriver(). The passed-in backend hint is replaced with the resolved backend.
-    static ExternalContext* create(driver::Backend* backendHint) noexcept;
-
     // Creates and and initializes the low-level API (e.g. an OpenGL context or Vulkan instance),
     // then creates the concrete Driver. Returns null on failure.
     virtual std::unique_ptr<Driver> createDriver(void* sharedGLContext) noexcept = 0;
@@ -50,6 +48,11 @@ public:
     virtual ~ExternalContext() noexcept;
 
     virtual int getOSVersion() const noexcept = 0;
+
+private:
+    friend class details::FEngine;
+    static ExternalContext* create(driver::Backend* backendHint) noexcept;
+    static void destroy(ExternalContext** context) noexcept;
 };
 
 class UTILS_PUBLIC ContextManagerGL : public ExternalContext {

filament/src/Engine.cpp

@@ -83,8 +83,13 @@ FEngine* FEngine::create(Backend backend, ExternalContext* externalContext, void
     // Normally we launch a thread and create the context and Driver from there (see FEngine::loop).
     // In the single-threaded case, we do so in the here and now.
     if (!UTILS_HAS_THREADING) {
-        instance->mExternalContext = ExternalContext::create(&instance->mBackend);
-        instance->mDriver = instance->mExternalContext->createDriver(sharedGLContext);
+        // we don't own the external context at that point, set it to null
+        instance->mExternalContext = nullptr;
+        if (externalContext == nullptr) {
+            externalContext = ExternalContext::create(&instance->mBackend);
+            instance->mExternalContext = externalContext;
+        }
+        instance->mDriver = externalContext->createDriver(sharedGLContext);
         instance->init();
         instance->execute();
         return instance;
@@ -315,10 +320,13 @@ void FEngine::shutdown() {
     if (UTILS_HAS_THREADING) {
         mDriverThread.join();
     }
-    mTerminated = true;
 
     // detach this thread from the jobsystem
     mJobSystem.emancipate();
+
+    ExternalContext::destroy(&mExternalContext);
+
+    mTerminated = true;
 }
 
 void FEngine::prepare() {
@@ -360,14 +368,19 @@ void FEngine::flush() {
 // -----------------------------------------------------------------------------------------------
 
 int FEngine::loop() {
-    if (mExternalContext == nullptr) {
-        mExternalContext = ExternalContext::create(&mBackend);
+    // we don't own the external context at that point, set it to null
+    ExternalContext* externalContext = mExternalContext;
+    mExternalContext = nullptr;
+
+    if (externalContext == nullptr) {
+        externalContext = ExternalContext::create(&mBackend);
+        mExternalContext = externalContext;
 #if !defined(NDEBUG)
         slog.d << "FEngine resolved backend: "
                << (mBackend == driver::Backend::VULKAN ? "Vulkan" : "OpenGL") << io::endl;
 #endif
     }
-    mDriver = mExternalContext->createDriver(mSharedGLContext);
+    mDriver = externalContext->createDriver(mSharedGLContext);
     mDriverBarrier.latch();
     if (UTILS_UNLIKELY(!mDriver)) {
         // if we get here, it's because the driver couldn't be initialized and the problem has

filament/src/driver/ExternalContext.cpp

@@ -62,6 +62,9 @@ ContextManagerGL::~ContextManagerGL() noexcept = default;
 
 ContextManagerVk::~ContextManagerVk() noexcept = default;
 
+// Creates the platform-specific ExternalContext object. The caller takes ownership and is
+// responsible for destroying it. Initialization of the backend API is deferred until
+// createDriver(). The passed-in backend hint is replaced with the resolved backend.
 ExternalContext* ExternalContext::create(Backend* backend) noexcept {
     assert(backend);
     if (*backend == Backend::DEFAULT) {
@@ -102,5 +105,11 @@ ExternalContext* ExternalContext::create(Backend* backend) noexcept {
     return nullptr;
 }
 
+// destroys an ExternalContext create by create()
+void ExternalContext::destroy(ExternalContext** context) noexcept {
+    delete *context;
+    *context = nullptr;
+}
+
 } // namespace driver
 } // namespace filament