dotnet-provoke
dotnet-provoke

Reputation: 1608

Should Semaphoreslim be in the controller instead of further down in the chain?

We have a deadlock like behaviour in our production-environment and I wonder if we use SemaphoreSlim correctly.

In our restapi the code looks like this:

public async Task<IActionResult> CreateAsync([FromBody] SomeModel someModel)
{
    var result = await _someClass.CreateArende(someModel);

    return result;
}

public async Task<IActionResult> RegisterAsync([FromBody] SomeModel someModel)
{
    var result = await _someClass.RegisterArende(someModel);

    return result;
}

No SemphoreSlim in controller level of our API but in someClass looks like this:

public class SomeClass
{
    protected static SemaphoreSlim _semphoreSlimCreateArende = new SemaphoreSlim(1, 1);

    public async virtual Task<SomeResponseDto> CreateArende(SomeModel someModel)
    {
        try
        {
            await _semphoreSlimCreateArende.WaitAsync();
        }
        finally
        {
            try
            {
                _semphoreSlimCreateArende.Release();
            }
            catch (Exception)
            {
            }
        }

        return new SomeResponseDto()
        {
            ...
        };
    }

    public async virtual Task<SomeResponseDto> RegisterArende(SomeModel someModel)
    {
        try
        {
            await _semphoreSlimCreateArende.WaitAsync();
        }
        finally
        {
            try
            {
                _semphoreSlimCreateArende.Release();
            }
            catch (Exception)
            {
            }
        }

        return new SomeResponseDto()
        {
            ...
        };
    }
}

Should the SemaphoreSlim be in the controller level instead? Or should I change the controllers actions to not be async?

Upvotes: 0

Views: 2147

Answers (1)

TheGeneral
TheGeneral

Reputation: 81523

This question is a little disjointed, however lets try to make sense of it a little

Firstly lets get the pattern right

try
{
    await _semphoreSlimCreateArende.WaitAsync();

    // do your sync work here

}
finally
{
    // if this throws you are certainly doing something wrong
    _semphoreSlimCreateArende.Release(); 
}

Secondly, you should have your keyboard taken away from you for this

catch (Exception)
{
}

Don't ever blindly eat exceptions, and if you are getting them on _semphoreSlimCreateArende.Release you have serious problems already and you have to work out why

Should the semaphoreslim be in the controller level instead?

Use them at the level that makes the most sense, i mean if you need to sync a piece of code sync it there, not 13 levels up.

Or should i change the controllers actions to not be async?

if you have asnyc work make your controller async and let it propagate down the stack to your async code

We have a deadlock like behaviour in our production-environment and i wonder if we use semaphoreslim correctly.

Woah, are we talking DataBase Deadlocks, or Context DeadLocks. Either way this all sounds a little fishy

Upvotes: 2

Related Questions