-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[release/6.0] Make sure that shared memory object name meets the length requiremens #64266
[release/6.0] Make sure that shared memory object name meets the length requiremens #64266
Conversation
dotnet#64099) Co-authored-by: Stephen Toub <stoub@microsoft.com> # Conflicts: # src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateNew.Tests.cs # src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewAccessor.Tests.cs # src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStream.Tests.cs # src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStreamConformanceTests.cs
Tagging subscribers to this area: @dotnet/area-system-io Issue DetailsA manual backport of #64099
|
/azp run runtime-extra-platforms |
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
return null; | ||
} | ||
Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); | ||
fd.Dispose(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, it's a little weird to Dispose of the SafeHandle and then continue to use it, but technically in this case it's ok to do so, given how it's being used. It's just unusual to see and thus a little disconcerting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a comment here would help clarify the unusual usage?
Could you add a template we could review, before tactics mail? |
Done |
Template looks good. I added that its customer reported. Feel free to send mail.. |
A manual backport of #64099
The sys-call that we use to create shared memory mapped objects (
shm_open
) has distro-specific map name length requirements. On most Unix-like Operating Systems it'sPATH_MAX
(4096), while on macOS it'sSHM_NAME_MAX
which for the arm64 version currently maps to 32. The code has been simply generating a string that is too long.Customer Impact
Customer reported #63240 in 6.0. macOS arm64 users can't create a memory mapped file which is not backed by a real file (
MemoryMappedFile.CreateNew(mapName: null)
) . There is no workaround.Testing
The tests have been.. re-enabled. Yes, we had failing tests that were clearly saying that there is a bug but they got disabled rather than fixed.
Risk
Low. By reducing the map name length we have reduced the entropy so chances for generating a name that is already in use have increased. However, a simple retry logic was added and it ensures that even if a shared map object with a given name already exists, a new name is generated and the process is repeated until it succeeds (or fails for other reasons).