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

[7.0] Fix session-local named mutex compat issue #90343

Merged
merged 2 commits into from
Sep 1, 2023

Conversation

kouvel
Copy link
Member

@kouvel kouvel commented Aug 10, 2023

  • Port of Fix session-local named mutex compat issue #90342
  • A previous change that was serviced back changed session-local named mutexes to be user-specific by restricting the permissions of the session directories and files under them, and adding the sticky bit to some directoires. A compat issue arose from that change, as the session directories have the session ID in their name and session IDs can be reused between different users. The current plan that we have discussed is to revert the change and service back the revert, which also restores the intended behavior, and offer user-specific mutexes as a new feature in a future .NET that would satisfy some user scenarios in a better way.
  • This PR reverts the previous change (first commit) and restores one change from the previous change (second commit) to improve backward compatibility due to differences in permissions for session directories before and after the change

Customer Impact

Files relevant to session-local named mutexes go into a session directory with the session ID in its name. Previously, the session directory's permissions were restricted to the creating user. When a session ID gets reused by a different user, a .NET process that tries to use a session-local named mutex fails with an exception because it's unable to create or access files under the session directory.

Regression?

Yes, from the PR being reverted (mentioned in the first commit)

Testing

Verified compatibility between unfixed and fixed versions of the runtime with using session-local named mutexes

Risk

Low. The compat issue may not be fully fixed in all cases since restricted permissions are involved, though based on the amount of feedback we've gotten about the compat issue, it seems likely that most cases would be resolved by updating the runtime to include this fix. There are one-time workarounds for remaining cases, such as updating all used runtime versions to include the fix, restarting the machine if the OS has a cleanup policy for /tmp/ upon restart, or deleting /tmp/.dotnet/shm/ and /tmp/.dotnet/lockfiles/ when there are no .NET processes running.

@kouvel kouvel added this to the 7.0.x milestone Aug 10, 2023
@kouvel kouvel self-assigned this Aug 10, 2023
@ghost
Copy link

ghost commented Aug 10, 2023

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Port of Fix session-local named mutex compat issue #90342
  • A previous change that was serviced back changed session-local named mutexes to be user-specific by restricting the permissions of the session directories and files under them, and adding the sticky bit to some directoires. A compat issue arose from that change, as the session directories have the session ID in their name and session IDs can be reused between different users. The current plan that we have discussed is to revert the change and service back the revert, which also restores the intended behavior, and offer user-specific mutexes as a new feature in a future .NET that would satisfy some user scenarios in a better way.
  • This PR reverts the previous change (first commit) and restores one change from the previous change (second commit) to improve backward compatibility due to differences in permissions for session directories before and after the change
  • Fixes Restrict named mutex files permissions #78106

Customer Impact

Files relevant to session-local named mutexes go into a session directory with the session ID in its name. Previously, the session directory's permissions were restricted to the creating user. When a session ID gets reused by a different user, a .NET process that tries to use a session-local named mutex fails with an exception because it's unable to create or access files under the session directory.

Regression?

Yes, from the PR mentioned in the first commit

Testing

Verified compatibility between unfixed and fixed versions of the runtime with using session-local named mutexes

Risk

Low. The compat issue may not be fully fixed in all cases since restricted permissions are involved, though based on the amount of feedback we've gotten about the compat issue, it seems likely that most cases would be resolved by updating the runtime to include this fix. There are one-time workarounds for remaining cases, such as updating all used runtime versions to include the fix, restarting the machine if the OS has a cleanup policy for /tmp/ upon restart, or deleting /tmp/.dotnet/shm/ and /tmp/.dotnet/lockfiles/ when there are no .NET processes running.

Author: kouvel
Assignees: kouvel
Labels:

area-System.Threading

Milestone: 7.0.x

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@carlossanlop
Copy link
Member

Don't forget to add the servicing-consider label when ready. Today is code complete for the September Release, so this will have to wait until the October Release.

@carlossanlop
Copy link
Member

carlossanlop commented Aug 31, 2023

@kouvel can you please retarget the PR to the release/7.0-staging branch? In case you don't know, you can easily do this by clicking on the Edit button on the top right, then below the title editing textbox, you can select the target branch.

@carlossanlop carlossanlop added the Servicing-approved Approved for servicing release label Aug 31, 2023
@kouvel kouvel changed the base branch from release/7.0 to release/7.0-staging September 1, 2023 02:12
@kouvel
Copy link
Member Author

kouvel commented Sep 1, 2023

Retargeted to release/7.0-staging

@carlossanlop
Copy link
Member

Thank you for fixing this! This will clear up a lot of CI failures.

@carlossanlop carlossanlop merged commit c0deb4f into dotnet:release/7.0-staging Sep 1, 2023
110 of 114 checks passed
@kouvel kouvel deleted the NamedMutexFix7 branch September 11, 2023 19:33
@ghost ghost locked as resolved and limited conversation to collaborators Oct 12, 2023
@leecow leecow modified the milestones: 7.0.x, 7.0.13 Nov 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Threading Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants