commit 5f8117acd45348704629a8aa7bd2169a5ad6a547
author Cody Northrop <cnorthrop@google.com> Tue Sep 26 20:48:59 2023 -0600
committer Cody Northrop <cnorthrop@google.com> Wed Sep 27 12:13:52 2023 -0600
tree 27192aa0a73f58e39f8f9b24eb837e4829baa54a
parent d5fd0d953e557ad492d3a5358d69d54a42d56e3a

EGL: Close Multifile Blobcache files after mapping

When loading entries from disk, we don't need to keep the
files open after mapping their contents.

Per mmap documentation:

  After the mmap() call has returned, the file descriptor, fd, can
  be closed immediately without invalidating the mapping.

  https://man7.org/linux/man-pages/man2/mmap.2.html

This will prevent consuming excessive file descriptors, which
are a limited resource.

Added new test that ensures file descriptors do not remain open.

Test: libEGL_test, EGL_test, restricted_trace_perf.py
Bug: b/286809755
Change-Id: I6317fdbce340a8e7cbf3020ad41386cf9915dd2d

opengl/libs/EGL/MultifileBlobCache.cpp

diff --git a/opengl/libs/EGL/MultifileBlobCache.cpp b/opengl/libs/EGL/MultifileBlobCache.cpp
index 7ffdac7..ed3c616 100644
--- a/opengl/libs/EGL/MultifileBlobCache.cpp
+++ b/opengl/libs/EGL/MultifileBlobCache.cpp
@@ -48,9 +48,8 @@
 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 @@
                 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 @@
                     if (remove(fullPath.c_str()) != 0) {
                         ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno));
                     }
+                    close(fd);
                     continue;
                 }
 
@@ -161,6 +162,10 @@
                 // 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 @@
                     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 @@
         // 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;
         }