Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5f8117ac authored by Cody Northrop's avatar Cody Northrop
Browse files

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
parent d5fd0d95

opengl/libs/EGL/MultifileBlobCache.cpp

+11 −5
Original line number Diff line number Diff line
@@ -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;

        }

opengl/libs/EGL/MultifileBlobCache_test.cpp

+54 −0
Original line number Diff line number Diff line
@@ -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