Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

carlossanlop
Copy link

Approved API Proposal: https://github.com/dotnet/corefx/issues/41662
Related PR for EventWaitHandle: #42213
Related PR for Mutex: #42281

Description

We don't currently have a way to create a Semaphore with a given ACL in .NET Core. We can modify the ACL, but it would be more secure to have the proper ACL on the object from the start.

Customer impact

Before this change, customers had to create a Semaphore, then set its ACLs. This presents a few problems:

  • Potential security hole as semaphores can be accessed between creation and modification.
  • Porting difficulties as there isn't a 1-1 API replacement

This change addresses those problems by adding a new extension method that allows creating a Semaphore and ensuring the provided ACLs are set during creation.

We will try to merge this to 3.1 if time permits it.

@carlossanlop
Copy link
Author

Pending adding unit tests.

@maryamariyan
Copy link

Thank you for your contribution. As announced in dotnet/coreclr#27549 this repository will be moving to dotnet/runtime on November 13. If you would like to continue working on this PR after this date, the easiest way to move the change to dotnet/runtime is:

  1. In your corefx repository clone, create patch by running git format-patch origin
  2. In your runtime repository clone, apply the patch by running git apply --directory src/corefx <path to the patch created in step 1>

@carlossanlop carlossanlop marked this pull request as ready for review November 6, 2019 22:58
@carlossanlop carlossanlop modified the milestones: 3.1, 5.0 Nov 7, 2019
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

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

Looks good!

@carlossanlop
Copy link
Author

Thank you for your signoffs! I resolved the conflicts with master, will wait for the CI to finish, and then will merge.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants