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

ArgumentException when adding large number of subscriptions #17

Closed
sidhoda opened this issue May 4, 2016 · 5 comments
Closed

ArgumentException when adding large number of subscriptions #17

sidhoda opened this issue May 4, 2016 · 5 comments

Comments

@sidhoda
Copy link

sidhoda commented May 4, 2016

The following code will more often than not result in an ArgumentException being raised with the message "An item with the same key has already been added.":

var bus = BusSetup.StartWith<AsyncConfiguration>().Construct();

for (uint i = 0; i < 100000; i++)
{
    bus.Subscribe((int x) => Console.WriteLine(x));
}

Relevant part of the exception stack trace is

   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at MemBus.CompositeSubscription.JustAdd(ISubscription subscription)
   at MemBus.CompositeSubscription.Add(ISubscription subscription)
   at MemBus.CompositeResolver.Add(ISubscription subscription)
   at MemBus.Subscriber.Subscribe[M](Action`1 subscription, ISubscriptionShaper customization)
   at MemBus.Subscriber.Subscribe[M](Action`1 subscription)
   at MemBus.MessageObservable`1.Subscribe(IObserver`1 observer)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)

My guess is that two DisposableSubscription instances have the same hashcode, resulting in a collision when attempting to add it the subscription storage:

_subscriptions.Add(disposableSub.GetHashCode(), disposableSub);

Changing the subscriptions storage to HashSet would be a possible fix as the default IEqualityComparer will check for equality (in this case reference equality) when getting multiple elements with the same hash code.

@jkonecki
Copy link
Contributor

I'm getting the same exception with much lower number of subscriptions: ~20

@flq
Copy link
Owner

flq commented Nov 19, 2016

Hey, thanks for the PR, wondering why this was a Dictionary right now. Pre-Hashset-time?

How could you verify that this sorts you out?

I wanted to push a new version with netstandard as target, but I need to migrate the project.json stuff I have also.

@jkonecki
Copy link
Contributor

I've added a unit test that @sidhoda suggested.
I cannot get the Test project to compile on my machine due to project.json references issue. I hope it will work on yours.
I've updated my project and haven't noticed the exception anymore.

@flq
Copy link
Owner

flq commented Nov 22, 2016

Sorry that your PR is sitting there, but I tried to upgrade to the new csproj stuff from .NET Core and looks like I failed, so I am doing another version with the project.json stuff and then hopefully I have the stuff up and running again.

@flq
Copy link
Owner

flq commented Nov 26, 2016

This has been fixed with commit e28770a.

@flq flq closed this as completed Nov 26, 2016
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

No branches or pull requests

3 participants