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

Possible Race Condition in ModelObjectPool #123

Open
mksergiy opened this issue Nov 16, 2024 · 1 comment · May be fixed by #125
Open

Possible Race Condition in ModelObjectPool #123

mksergiy opened this issue Nov 16, 2024 · 1 comment · May be fixed by #125

Comments

@mksergiy
Copy link

mksergiy commented Nov 16, 2024

A possible race condition exists in the ModelObjectPool implementation when multiple threads concurrently call the Return method. The issue arises due to the non-atomic _availableObjects.Count check and the subsequent _availableObjects.Add().
In my specific use case, this issue has resulted in RabbitMQ channel leaks.

I fixed it by replacing the _availableObjects.Count logic with an atomic counter (_currentCount) managed using Interlocked. Here's the sample:

private int _currentCount = 0;

public void Return(IModel model)
{
    if (Interlocked.Increment(ref _currentCount) > _maxEntries)
    {
        Interlocked.Decrement(ref _currentCount);
        model.SafeDrop();
    }
    else
    {
        _availableObjects.Add(model);
    }
}
public IModel Get()
{
    if (_availableObjects.TryTake(out var model))
    {
        Interlocked.Decrement(ref _currentCount);
        return model;
    }

    return _policy.Create();
}
@mookid8000
Copy link
Member

Oh! I appreciate your report and the fact that you submitted the code.... but how about submitting it as a PR? Then you'll get your name in the list of commits 🤠 and you will be a Rebus contributor 4ever 🤩

@mksergiy mksergiy linked a pull request Dec 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants