2 files changed, 65 insertions, 5 deletions

diff --git a/opengl/libs/EGL/MultifileBlobCache.cpp b/opengl/libs/EGL/MultifileBlobCache.cpp
index 7ffdac7ea7..ed3c616b92 100644
--- a/opengl/libs/EGL/MultifileBlobCache.cpp
+++ b/opengl/libs/EGL/MultifileBlobCache.cpp
@@ -48,9 +48,8 @@ namespace {
 void freeHotCacheEntry(android::MultifileHotCache& entry) {
     if (entry.entryFd != -1) {
         // If we have an fd, then this entry was added to hot cache via INIT or GET
-        // We need to unmap and close the entry
+        // We need to unmap the entry
         munmap(entry.entryBuffer, entry.entrySize);
-        close(entry.entryFd);
     } else {
         // Otherwise, this was added to hot cache during SET, so it was never mapped
         // and fd was only on the deferred thread.
@@ -143,6 +142,7 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s
                 if (result != sizeof(MultifileHeader)) {
                     ALOGE("Error reading MultifileHeader from cache entry (%s): %s",
                           fullPath.c_str(), std::strerror(errno));
+                    close(fd);
                     return;
                 }
 
@@ -152,6 +152,7 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s
                     if (remove(fullPath.c_str()) != 0) {
                         ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno));
                     }
+                    close(fd);
                     continue;
                 }
 
@@ -161,6 +162,10 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s
                 // Memory map the file
                 uint8_t* mappedEntry = reinterpret_cast<uint8_t*>(
                         mmap(nullptr, fileSize, PROT_READ, MAP_PRIVATE, fd, 0));
+
+                // We can close the file now and the mmap will remain
+                close(fd);
+
                 if (mappedEntry == MAP_FAILED) {
                     ALOGE("Failed to mmap cacheEntry, error: %s", std::strerror(errno));
                     return;
@@ -206,13 +211,11 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s
                     if (!addToHotCache(entryHash, fd, mappedEntry, fileSize)) {
                         ALOGE("INIT Failed to add %u to hot cache", entryHash);
                         munmap(mappedEntry, fileSize);
-                        close(fd);
                         return;
                     }
                 } else {
                     // If we're not keeping it in hot cache, unmap it now
                     munmap(mappedEntry, fileSize);
-                    close(fd);
                 }
             }
             closedir(dir);
@@ -401,9 +404,12 @@ EGLsizeiANDROID MultifileBlobCache::get(const void* key, EGLsizeiANDROID keySize
         // Memory map the file
         cacheEntry =
                 reinterpret_cast<uint8_t*>(mmap(nullptr, fileSize, PROT_READ, MAP_PRIVATE, fd, 0));
+
+        // We can close the file now and the mmap will remain
+        close(fd);
+
         if (cacheEntry == MAP_FAILED) {
             ALOGE("Failed to mmap cacheEntry, error: %s", std::strerror(errno));
-            close(fd);
             return 0;
         }
 
diff --git a/opengl/libs/EGL/MultifileBlobCache_test.cpp b/opengl/libs/EGL/MultifileBlobCache_test.cpp
index dbee13bb2e..1639be6480 100644
--- a/opengl/libs/EGL/MultifileBlobCache_test.cpp
+++ b/opengl/libs/EGL/MultifileBlobCache_test.cpp
@@ -42,6 +42,8 @@ protected:
 
     virtual void TearDown() { mMBC.reset(); }
 
+    int getFileDescriptorCount();
+
     std::unique_ptr<TemporaryFile> mTempFile;
     std::unique_ptr<MultifileBlobCache> mMBC;
 };
@@ -216,4 +218,56 @@ TEST_F(MultifileBlobCacheTest, CacheMinKeyAndValueSizeSucceeds) {
     ASSERT_EQ('y', buf[0]);
 }
 
+int MultifileBlobCacheTest::getFileDescriptorCount() {
+    DIR* directory = opendir("/proc/self/fd");
+
+    int fileCount = 0;
+    struct dirent* entry;
+    while ((entry = readdir(directory)) != NULL) {
+        fileCount++;
+        // printf("File: %s\n", entry->d_name);
+    }
+
+    closedir(directory);
+    return fileCount;
+}
+
+TEST_F(MultifileBlobCacheTest, EnsureFileDescriptorsClosed) {
+    // Populate the cache with a bunch of entries
+    size_t kLargeNumberOfEntries = 1024;
+    for (int i = 0; i < kLargeNumberOfEntries; i++) {
+        // printf("Caching: %i", i);
+
+        // Use the index as the key and value
+        mMBC->set(&i, sizeof(i), &i, sizeof(i));
+
+        int result = 0;
+        ASSERT_EQ(sizeof(i), mMBC->get(&i, sizeof(i), &result, sizeof(result)));
+        ASSERT_EQ(i, result);
+    }
+
+    // Ensure we don't have a bunch of open fds
+    ASSERT_LT(getFileDescriptorCount(), kLargeNumberOfEntries / 2);
+
+    // Close the cache so everything writes out
+    mMBC->finish();
+    mMBC.reset();
+
+    // Now open it again and ensure we still don't have a bunch of open fds
+    mMBC.reset(
+            new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, &mTempFile->path[0]));
+
+    // Check after initialization
+    ASSERT_LT(getFileDescriptorCount(), kLargeNumberOfEntries / 2);
+
+    for (int i = 0; i < kLargeNumberOfEntries; i++) {
+        int result = 0;
+        ASSERT_EQ(sizeof(i), mMBC->get(&i, sizeof(i), &result, sizeof(result)));
+        ASSERT_EQ(i, result);
+    }
+
+    // And again after we've actually used it
+    ASSERT_LT(getFileDescriptorCount(), kLargeNumberOfEntries / 2);
+}
+
 } // namespace android