checkout / checkout-sdk-net

Checkout SDK for Microsoft .NET
https://checkout.com
MIT License
21 stars 23 forks source link

LogProvider.GetLogger method occasionally throws InvalidOperationException due to concurrent writes #363

Closed IanKemp closed 6 months ago

IanKemp commented 10 months ago

Environment

Description

The LogProvider class internally maintains a Dictionary<string, ILogger> holding loggers created by its GetLogger method. Because Dictionary is not inherently thread-safe, it is possible for thread A to be attempting to fetch a logger from this dictionary at the same time that thread B is adding one, which causes an InvalidOperationException to be thrown for A:

   System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.

   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.set_Item(TKey key, TValue value)
   at Checkout.LogProvider.GetLogger(Type clazz)
   at Checkout.ApiClient..ctor(IHttpClientFactory httpClientFactory, Uri baseUri)
   at Checkout.CheckoutApi.BaseApiClient(CheckoutConfiguration configuration)
   at Checkout.CheckoutApi..ctor(CheckoutConfiguration configuration)
   at Checkout.CheckoutSdkBuilder.CheckoutStaticKeysSdkBuilder.Build()

We have encountered this issue extremely intermittently, but often enough that it should be addressed.

Expected behavior

This exception should never be thrown from within this method.

Current behavior

As above.

Steps to reproduce

Use the SDK often enough and you'll hit this.

Possible solution

This can easily be fixed by changing the type of the internal logger collection from Dictionary to ConcurrentDictionary, which is designed to support this usage pattern.

Additional information

N/A

armando-rodriguez-cko commented 10 months ago

Thank you @IanKemp! We will fix it in the new release.

armando-rodriguez-cko commented 8 months ago

Hi @IanKemp, we are testing your proposal. Thank you