Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MemoryMappedView.CreateView - better error message for insufficient virtual memory #22846

Open
dchennells opened this issue Jul 18, 2017 · 0 comments
Labels
area-System.IO enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@dchennells
Copy link

The static factory method of the internal MemoryMappedView is typically called from MemoryMappedFile.CreateViewAccessor(...) after a user has created a MemoryMappedFile. An overload of CreateViewAccessor() with no parameters is provided for the common case that the view is to cover the entire file. That overload results in a DefaultSize value of zero being passed to the size parameter of MemoryMappedView.CreateView().

Line 38 of the file MemoryMappedView.Windows.cs calls a check, preceded by a comment:

            // if request is >= than total virtual, then MapViewOfFile will fail with meaningless error message 
            // "the parameter is incorrect"; this provides better error message in advance
            Interop.CheckForAvailableVirtualMemory(nativeSize);

The problem is that "nativeSize" as implemented is zero under the (most?) common case just mentioned, such that the check (in Interop.Windows.cs) is not very probative:

        if (nativeSize >= totalVirtual)
        {
            throw new IOException(SR.IO_NotEnoughMemory);
        }

Now the one caller (MemoryMappedFile) of MemoryMappedView.CreateView() has the actual file length (from its FileStream, or from a native call). On line 152 of MemoryMappedFile it ensures that the filestream.length is meaningful under the default capacity/size scenario, so it could readily provide it to MemoryMappedView.CreateView() to enable an accurate lower-bound expected memory consumption calculation and check before the Pinvoke call to MapViewOfFile. (By considering both the system page size as well as the allocation granularity it is straightforward to calculate a value that matches the viewInfo.RegionSize that is later returned by the existing VirtualQuery call.)

Given that this problem is not resulting in a catastrophic failure (only a "meaningless error message," per the comment in the code) and that it may not be all that common to be mapping views of large files exceeding the available virtual memory, I'm unsure where this fits in the grand scheme of priorities. However, I wanted to log it given that I found myself adding a remedy today in my own specialized implementation in case someone eventually logs a bug on this. I'm new here, almost a first time participant, so please let me know to what extent this falls below (or above) the current level of relevance/urgency.

@karelz karelz changed the title System.IO.MemoryMappedFiles.MemoryMappedView.CreateView(...) check for sufficient virtual memory is incomplete MemoryMappedFiles.MemoryMappedView.CreateView check for sufficient virtual memory is incomplete Jan 22, 2020
@karelz karelz changed the title MemoryMappedFiles.MemoryMappedView.CreateView check for sufficient virtual memory is incomplete MemoryMappedView.CreateView - better error message for insufficient virtual memory Jan 22, 2020
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@JeremyKuhne JeremyKuhne removed the untriaged New issue has not been triaged by the area owner label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.IO enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants