From b9efd4fbf03f4c3beebcf7776d8a450a4996fd3d Mon Sep 17 00:00:00 2001
From: Philip Rideout <philiprideout@gmail.com>
Date: Mon, 15 Aug 2022 13:53:46 -0700
Subject: [PATCH] gltfio Android: Fix double free error.

The custom release callback that I provided was in the wrong place; it
was only being used for a path string, not the actual buffer content.
The cgltf API is a bit awkward in this area.

No need to update RELEASE_NOTES because this will be cherry picked to
the RC branch, which is where the bug introduced.

Fixes #5918.
---
 libs/gltfio/src/AssetLoader.cpp    | 15 +++++++++++++++
 libs/gltfio/src/FFilamentAsset.h   |  6 ++++++
 libs/gltfio/src/ResourceLoader.cpp | 21 ++++++++-------------
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/libs/gltfio/src/AssetLoader.cpp b/libs/gltfio/src/AssetLoader.cpp
index 7ef5259469..5759ad8c12 100644
--- a/libs/gltfio/src/AssetLoader.cpp
+++ b/libs/gltfio/src/AssetLoader.cpp
@@ -174,6 +174,21 @@ FILAMENT_UPCAST(AssetLoader)
 
 FFilamentAsset* FAssetLoader::createAssetFromJson(const uint8_t* bytes, uint32_t nbytes) {
     cgltf_options options { cgltf_file_type_invalid };
+
+    if constexpr (!GLTFIO_USE_FILESYSTEM) {
+
+        // Provide a custom free callback for each buffer that was loaded from a "file", as opposed
+        // to a data:// URL.
+        //
+        // Since GLTFIO_USE_FILESYSTEM is false, ResourceLoader requires the app provide the file
+        // content from outside, so we need to do nothing here, as opposed to the default, which is
+        // to call "free".
+        //
+        // This callback also gets called for the root-level file_data, but since we use
+        // `cgltf_parse`, the file_data field is always null.
+        options.file.release = [](const cgltf_memory_options*, const cgltf_file_options*, void*) {};
+    }
+
     cgltf_data* sourceAsset;
     cgltf_result result = cgltf_parse(&options, bytes, nbytes, &sourceAsset);
     if (result != cgltf_result_success) {
diff --git a/libs/gltfio/src/FFilamentAsset.h b/libs/gltfio/src/FFilamentAsset.h
index f32357522a..a2aee229a9 100644
--- a/libs/gltfio/src/FFilamentAsset.h
+++ b/libs/gltfio/src/FFilamentAsset.h
@@ -57,6 +57,12 @@
 #define GLTFIO_WARN(msg) slog.w << msg << io::endl
 #endif
 
+#if defined(__EMSCRIPTEN__) || defined(__ANDROID__) || defined(IOS)
+#define GLTFIO_USE_FILESYSTEM 0
+#else
+#define GLTFIO_USE_FILESYSTEM 1
+#endif
+
 namespace utils {
     class NameComponentManager;
     class EntityManager;
diff --git a/libs/gltfio/src/ResourceLoader.cpp b/libs/gltfio/src/ResourceLoader.cpp
index b163c732cc..0d6f63a2f9 100644
--- a/libs/gltfio/src/ResourceLoader.cpp
+++ b/libs/gltfio/src/ResourceLoader.cpp
@@ -48,12 +48,6 @@
 #include <string>
 #include <fstream>
 
-#if defined(__EMSCRIPTEN__) || defined(__ANDROID__) || defined(IOS)
-#define USE_FILESYSTEM 0
-#else
-#define USE_FILESYSTEM 1
-#endif
-
 using namespace filament;
 using namespace filament::math;
 using namespace utils;
@@ -390,7 +384,7 @@ bool ResourceLoader::loadResources(FFilamentAsset* asset, bool async) {
     // cache of externally-supplied data blobs, rather than loading from the filesystem.
 
     SYSTRACE_NAME_BEGIN("Load buffers");
-    #if !USE_FILESYSTEM
+    #if !GLTFIO_USE_FILESYSTEM
 
     struct Closure {
         Impl* impl;
@@ -410,13 +404,14 @@ bool ResourceLoader::loadResources(FFilamentAsset* asset, bool async) {
         if (auto iter = uriDataCache.find(path); iter != uriDataCache.end()) {
             *size = iter->second.size;
             *data = iter->second.buffer;
+        } else {
+            // Even if we don't find the given resource in the cache, we still return a successful
+            // error code, because we allow downloads to finish after the decoding work starts.
+           *size = 0;
+           *data = 0;
         }
-        return cgltf_result_success;
-    };
 
-    options.file.release = [](const cgltf_memory_options* memoryOpts,
-            const cgltf_file_options* fileOpts, void* data) {
-        // Do nothing here because no memory was allocated in the read callback.
+        return cgltf_result_success;
     };
 
     #endif
@@ -673,7 +668,7 @@ Texture* ResourceLoader::Impl::getOrCreateTexture(FFilamentAsset* asset, const T
     }
 
     // Finally, try the file system.
-    else if constexpr (USE_FILESYSTEM) {
+    else if constexpr (GLTFIO_USE_FILESYSTEM) {
         if (auto iter = mFilepathTextureCache.find(uri); iter != mFilepathTextureCache.end()) {
             return iter->second;
         }