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

SemaphoreSlim is not work ? #21

Closed
juner opened this Issue May 30, 2018 · 9 comments

Comments

Projects
None yet
2 participants
@juner

juner commented May 30, 2018

The following code does not work with Theraot.Core in .net 3.5.
Do you know what is the cause?
https://gist.github.com/juner/cee3c83360f8d5434e0ebb68cda63294

In addition, the original source was as follows.
https://docs.microsoft.com/en-us/dotnet/api/system.threading.semaphoreslim

@theraot

This comment has been minimized.

Owner

theraot commented May 30, 2018

@juner

This comment has been minimized.

juner commented May 30, 2018

@theraot If so, is SemaphoreSlim not incrementing beyond the set maximum?

@theraot

This comment has been minimized.

Owner

theraot commented May 31, 2018

@theraot

This comment has been minimized.

Owner

theraot commented May 31, 2018

I am calling this fixed

I used the following code (based on your gist) to verify:

using System;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace ConsoleApp1
{
    internal class Program
    {
        private static void Main()
        {
            var max = 3;
            var taskCount = 3;
            var log = new Theraot.Collections.ThreadSafe.CircularBucket<string>(taskCount * 6 + 8);
            using (var source = new CancellationTokenSource(TimeSpan.FromSeconds(100)))
            {
                using (var semaphore = new SemaphoreSlim(0, max))
                {
                    var padding = 0;
                    var passCount = 0;
                    var fail = 0;
                    log.Add($"[Before Begin] PassCount = {Volatile.Read(ref passCount)}");
                    var tasks = Enumerable.Range(0, taskCount).Select
                    (
                        index =>
                        {
                            return Task.Factory.StartNew
                            (
                                async () =>
                                {
                                    log.Add($"[Task: {index}, Created, Waiting] PassCount = {Volatile.Read(ref passCount)}");
                                    await semaphore.WaitAsync(source.Token);
                                    Thread.MemoryBarrier();
                                    Interlocked.Add(ref padding, 100);
                                    var found = Interlocked.Increment(ref passCount);
                                    log.Add($"[Task: {index}, Enter] PassCount = {found}, semaphore.CurrentCount = {semaphore.CurrentCount}");
                                    if (found > max)
                                    {
                                        log.Add($"[Task: {index}]: Fail (incremented beyond max)");
                                        Interlocked.Increment(ref fail);
                                    }
                                    await Task.Delay(1000 + padding);
                                    found = Thread.VolatileRead(ref passCount);
                                    log.Add($"[Task: {index}, Before Release] PassCount = {found}, semaphore.CurrentCount = {semaphore.CurrentCount}");
                                    if (found > max)
                                    {
                                        log.Add($"[Task: {index}]: Fail (incremented beyond max)");
                                        Interlocked.Increment(ref fail);
                                    }
                                    found = Interlocked.Decrement(ref passCount);
                                    var count = semaphore.Release();
                                    log.Add($"[Task: {index}, Release] PassCount = {found}, Previous Count = {count}");
                                }
                            ).Unwrap();
                        }
                    ).ToArray();
                    Thread.Sleep(TimeSpan.FromMilliseconds(500));
                    var foundPassCount = Volatile.Read(ref passCount);
                    log.Add($"[All Tasks Started, Before Release] PassCount = {foundPassCount}");
                    if (foundPassCount > max || fail > 0)
                    {
                        log.Add($"Fail: foundPassCount = {foundPassCount}, fail = {fail}");
                    }
                    semaphore.Release(max);
                    foundPassCount = Volatile.Read(ref passCount);
                    log.Add($"[After Release, Before WaitAll] PassCount = {foundPassCount}, semaphore.CurrentCount = {semaphore.CurrentCount}");
                    if (foundPassCount > max || fail > 0)
                    {
                        log.Add($"Fail: foundPassCount = {foundPassCount}, fail = {fail}");
                    }
                    Task.WaitAll(tasks, source.Token);
                    foundPassCount = Volatile.Read(ref passCount);
                    log.Add($"[After WaitAll] PassCount = {foundPassCount}");
                    if (foundPassCount > max || fail > 0)
                    {
                        log.Add($"Fail: foundPassCount = {foundPassCount}, fail = {fail}");
                    }
                    log.Add($"[Done]");
                    foreach (var entry in log)
                    {
                        Console.WriteLine(entry);
                    }
                    Console.ReadKey();
                }
            }
        }
    }
}

I am pushing to stable

@juner

This comment has been minimized.

juner commented Jun 5, 2018

https://gist.github.com/juner/cee3c83360f8d5434e0ebb68cda63294

.NET 4.5

0 task can enter the semaphore.
Task 5 begins and waits for the semaphore.
Task 1 begins and waits for the semaphore.
Task 3 begins and waits for the semaphore.
Task 7 begins and waits for the semaphore.
Main thread call Release(3) -->
0 tasks can enter the semaphore.
Task 3 enters the semaphore.
Task 5 enters the semaphore.
Task 1 enters the semaphore.
Task 5 release the semaphore; previous count: 0 
Task 1 release the semaphore; previous count: 1 
Task 7 enters the semaphore.
Task 3 release the semaphore; previous count: 0 
Task 7 release the semaphore; previous count: 2 

Since [3], [5] and [1] are entering, [7] can not enter until one is released

.NET 3.5 + Theraot.Core 1.0.4-pre20180531

0 task can enter the semaphore.
Task 2 begins and waits for the semaphore.
Task 0 begins and waits for the semaphore.
Task 4 begins and waits for the semaphore.
Task 6 begins and waits for the semaphore.
Main thread call Release(3) -->
Task 6 enters the semaphore.
Task 0 enters the semaphore.
Task 4 enters the semaphore.
Task 2 enters the semaphore.
Task 2 release the semaphore; previous count: 2 
Task 4 release the semaphore; previous count: 2 
Task 0 release the semaphore; previous count: 2 
Task 6 release the semaphore; previous count: 2 
3 tasks can enter the semaphore.
Main thread exits

why all enters? not wait?

@theraot

This comment has been minimized.

Owner

theraot commented Jun 5, 2018

@theraot

This comment has been minimized.

Owner

theraot commented Jun 5, 2018

@juner I have a fix, hopefully definitive.

This test is based on your code: https://github.com/theraot/Theraot/blob/v1/Tests/System/Threading/SemaphoreSlimTestsEx.cs

Passing.

Pushing to nuget soon.

Errata: nope, fails sometimes :( - can't push this.

@theraot

This comment has been minimized.

Owner

theraot commented Jun 8, 2018

@juner A new build just went up to nuget.

I will double check with the nuget package. In addition, I'll see if I can improve the the perfomance. So, there might be another nuget afterwards.

@theraot

This comment has been minimized.

Owner

theraot commented Jun 8, 2018

@juner A new stable is up.

Example output with this version:

0 task can enter the semaphore.
Task 7 begins and waits for the semaphore.
Task 1 begins and waits for the semaphore.
Task 5 begins and waits for the semaphore.
Task 3 begins and waits for the semaphore.
Main thread call Release(3) -->
2 tasks can enter the semaphore.
Task 5 enters the semaphore.
Task 1 enters the semaphore.
Task 7 enters the semaphore.
Task 5 release the semaphore; previous count: 0
Task 3 enters the semaphore.
Task 1 release the semaphore; previous count: 0
Task 7 release the semaphore; previous count: 1
Task 3 release the semaphore; previous count: 2
Main thread exits

I am closing this issue.

@theraot theraot closed this Jun 8, 2018

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