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

Problems with SemaphoreSlim on OSX #31502

Closed
johnnyasantoss opened this issue Nov 15, 2019 · 4 comments
Closed

Problems with SemaphoreSlim on OSX #31502

johnnyasantoss opened this issue Nov 15, 2019 · 4 comments
Labels
area-System.Threading untriaged New issue has not been triaged by the area owner

Comments

@johnnyasantoss
Copy link

Hello,
I noticed that SemaphoreSlim.WaitAsync(int) is sometimes extremely slow on OSX.

I was trying to set up a test and build a R2R version of a console app on Azure Pipelines, and noticed that the MacOS tests were failing on a simple wait & release test (logs).

So, I created a simplified version of that test and tried to run again using the same matrix.
Here's the repro, and here is the pipeline with the failed jobs on OSX.

BTW I'm using Linux and I don't have a machine with OSX available to test this.

This issue might be related to #25790

@stephentoub
Copy link
Member

(This looks buggy:
https://github.com/johnnyasantoss/semaphore-slim-osx-repro/blob/2aa38e6f27176787cc5c7f1f69cf6f4d006dd86c/tests/SemaphreRepro.Tests/UnitTest1.cs#L20-L28
You're creating the semaphore with a max count of 1. If that Release in the thread you've started is able to execute twice before the consumer waits on the semaphore, or more generally if it gets ahead of the consumer by even a small amount, that Release call is going to throw an exception when it tries to increment the count beyond what's allowed. That's probably unrelated, of course; if it were the issue, I'd actually expect your test to crash rather than fail due to a timeout.)

If I'm understanding your concern correctly, it's that you're having a thread sleep for 100ms between releases, and you're seeing waits on the semaphore take upwards of 172ms. Correct? Have you timed the actual duration of the Thread.Sleep in the instance where the wait on the semaphore takes too long? Have you tried using something other than a SemaphoreSlim?

I suspect this isn't actually to do with SemaphoreSlim, and rather is "just" the nature of timings and thread scheduling on macOS. Potentially related to https://github.com/dotnet/coreclr/issues/20369.

@johnnyasantoss
Copy link
Author

(This looks buggy:
https://github.com/johnnyasantoss/semaphore-slim-osx-repro/blob/2aa38e6f27176787cc5c7f1f69cf6f4d006dd86c/tests/SemaphreRepro.Tests/UnitTest1.cs#L20-L28
You're creating the semaphore with a max count of 1. If that Release in the thread you've started is able to execute twice before the consumer waits on the semaphore, or more generally if it gets ahead of the consumer by even a small amount, that Release call is going to throw an exception when it tries to increment the count beyond what's allowed. That's probably unrelated, of course; if it were the issue, I'd actually expect your test to crash rather than fail due to a timeout.)

Yep its buggy, but it is just to demonstrate the delay between the release and the actual release of the semaphore.

If I'm understanding your concern correctly, it's that you're having a thread sleep for 100ms between releases, and you're seeing waits on the semaphore take upwards of 172ms. Correct?

Yes

Have you timed the actual duration of the Thread.Sleep in the instance where the wait on the semaphore takes too long? Have you tried using something other than a SemaphoreSlim?

Not yet, but will do.

I suspect this isn't actually to do with SemaphoreSlim, and rather is "just" the nature of timings and thread scheduling on macOS. Potentially related to dotnet/coreclr#20369.

Thanks, I'll investigate this further and time the Thread.Sleeps in the tests.

@johnnyasantoss
Copy link
Author

Apparently you were right. The Thread.Sleep calls are taking a lot more time than it should. (see this log)

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@stephentoub
Copy link
Member

Closing as dup of #11226

@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Threading untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

3 participants