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

Disable saving source texts to memory mapped files on non-Windows platforms #73628

Closed
wants to merge 2 commits into from

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented May 22, 2024

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels May 22, 2024
@CyrusNajmabadi CyrusNajmabadi changed the title Disable saving source texts to memory mapped files on non-Windows pla… Disable saving source texts to memory mapped files on non-Windows platforms May 22, 2024
@CyrusNajmabadi
Copy link
Member Author

@dibarbet ptal

Copy link
Member

@sharwell sharwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be really handled by the storage service itself, i.e.:

// MemoryMapped files which are used by the TemporaryStorageService are present in .NET Framework (including Mono)
// and .NET Core Windows. For non-Windows .NET Core scenarios, we can return the TrivialTemporaryStorageService
// until https://github.com/dotnet/runtime/issues/30878 is fixed.
return PlatformInformation.IsWindows || PlatformInformation.IsRunningOnMono
? new TemporaryStorageService(workspaceThreadingService, textFactory)
: TrivialTemporaryStorageService.Instance;

@sharwell
Copy link
Member

Why is this reaching an AccessViolationException, as opposed to a PlatformNotSupportedException/NotSupportedException/NotImplementedException somewhere before that?

@CyrusNajmabadi
Copy link
Member Author

Mmfs are supported. The issue is that you can literally run out of space with them on Linux. This is happening within a container where the total amount of mmf space is limited (to like 32mb). So it's more like it's a scarce resource there, not that it isn't supported.

@sharwell
Copy link
Member

In that case, it should be an IOException, but the point still holds. Something is assuming a pointer is valid when it is not.

@CyrusNajmabadi
Copy link
Member Author

In that case, it should be an IOException, but the point still holds. Something is assuming a pointer is valid when it is not.

@sharwell The call to create the MMF succeeds. But writing it fails on linux when it actually tries to write to the portion of the mapped file that exceeds the allowed size of the environment.

@mjrist
Copy link

mjrist commented Jul 10, 2024

Any update on this? We are working around this by increasing the shared mem limit as described here (dotnet/vscode-csharp#7119 (comment)), but would prefer not to do that.

@dibarbet
Copy link
Member

Any update on this? We are working around this by increasing the shared mem limit as described here (dotnet/vscode-csharp#7119 (comment)), but would prefer not to do that.

Had to take a slightly different approach - the new version of the fix is here - #74339

@CyrusNajmabadi
Copy link
Member Author

Closing out in favor of David's solution.

@CyrusNajmabadi CyrusNajmabadi deleted the linuxMMF branch July 11, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
4 participants