MemoryMappedFile unnecessarily holding files open?

MacOSX and iOS
1

I’ve been working on the iOS version of an application that uses a large number MemoryMappedAudioFormatReaders and have run into a problem where after successfully mapping many files, files start failing to open with the error ENFILE here, indicating that it’s hit the max number of open files allowed (default of 256 on iOS I believe). After some digging, I found that the MemoryMappedFiles created by the MemoryMappedAudioFormatReaders hold files open for their lifetimes, resulting in this problem.

Would it be possible to change MemoryMappedFile so that it closes its file after mapping in MemoryMappedFile::openInternal, or is there a reason to keep the file open?

I have tried out this change myself (just copying lines 587-588 of juce_posix_SharedCode.h and inserting that after line 578) and it does seem to solve my problem, but I’m unsure whether this could have consequences elsewhere. The code in question:

void MemoryMappedFile::openInternal (const File& file, AccessMode mode, bool exclusive)
{
    jassert (mode == readOnly || mode == readWrite);

    if (range.getStart() > 0)
    {
        auto pageSize = sysconf (_SC_PAGE_SIZE);
        range.setStart (range.getStart() - (range.getStart() % pageSize));
    }

    auto filename = file.getFullPathName().toUTF8();

    if (mode == readWrite)
        fileHandle = open (filename, O_CREAT | O_RDWR, 00644);
    else
        fileHandle = open (filename, O_RDONLY);

    if (fileHandle != -1)
    {
        auto m = mmap (nullptr, (size_t) range.getLength(),
                       mode == readWrite ? (PROT_READ | PROT_WRITE) : PROT_READ,
                       exclusive ? MAP_PRIVATE : MAP_SHARED, fileHandle,
                       (off_t) range.getStart());

        if (m != MAP_FAILED)
        {
            address = m;
            madvise (m, (size_t) range.getLength(), MADV_SEQUENTIAL);
        }
        else
        {
            range = Range<int64>();
        }   
        // INSERT HERE
    }
}

MemoryMappedFile::~MemoryMappedFile()
{
    if (address != nullptr)
        munmap (address, (size_t) range.getLength());

    if (fileHandle != 0)     // COPY
        close (fileHandle);  // THESE
}

The implementation of MemoryMappedFile in juce_win32_Files.cpp already appears to close files after mapping, and the documentation of mmap here also suggests to me that it’s safe to close the file once mapped.

The mmap function creates a new mapping, connected to bytes (offset) to (offset + length - 1) in the file open on filedes. A new reference for the file specified by filedes is created, which is not removed by closing the file.

2

Is it possible to have the change Matt suggests here made to the JUCE source, assuming it’s ok?

3

Thanks for reporting. We’ve added this to develop here:

The Linux and POSIX docs for mmap both state that closing the file handle won’t unmap the memory region so it’s a safe change.

1 Like