ArgumentException when adding large number of subscriptions
Closed this issue · 5 comments
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:
MemBus/MemBus/CompositeSubscription.cs
Line 99 in 8613a7e
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.
I'm getting the same exception with much lower number of subscriptions: ~20
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.
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.
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.