Fix a race condition in ReconfigureTenant

[nmoATusercube]

ReconfigureTenant removes the tenant scope before adding the reconfigured one. If another thread calls GetTenantScope between the remove and the add, an empty scope is kept in the _tenantLifetimeScopes dictionary.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1e4c063) 90.62% compared to head (d902b77) 93.57%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop      #38      +/-   ##
===========================================
+ Coverage    90.62%   93.57%   +2.95%     
===========================================
  Files            5        5              
  Lines           96      109      +13     
  Branches        27       28       +1     
===========================================
+ Hits            87      102      +15     
+ Misses           4        3       -1     
+ Partials         5        4       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tillig]

While I see what this is trying to do, part of the challenge here is that there's an assumption that configuring Autofac is thread-safe. Component registration and container building have never been thread-safe.

There's going to be the same race condition if someone is calling DisposeAsync on one thread while ReconfigureTenant is called on another.

We used to have reader/writer locking (#8) but that got removed because it didn't work with DisposeAsync (#35) (as an example of why this may not be as easy as all that).

Anyway, that's what I'm thinking about.

I wonder if this would be a good use of ConcurrentDictionary.AddOrUpdate or maybe ConcurrentDictionary.TryUpdate - ways to use the locking inside ConcurrentDictionary so we don't have to kind of hack around it.

[nmoATusercube]

I completely agree. The first solution aimed at minimize the amount of changes while making ReconfigureTenant deterministic.
The new commit implements your suggestion of AddOrUpdate to atomically change the tenants dictionary while respecting the return value behavior.

[tillig]

This is looking nice, I like the updated use of the ConcurrentDictionary bits. I have a couple of inline comments, but minor stuff. Thanks!

[nmoATusercube]

I have also added a test for the case where we go through the Add path in the AddOrUpdate.

[tillig]

This is awesome. Let me get this merged in and I'll make a 0.0.1 release with it.