configcat / .net-sdk

ConfigCat SDK for .NET. ConfigCat is a hosted feature flag service: https://configcat.com. Manage feature toggles across frontend, backend, mobile, desktop apps. Alternative to LaunchDarkly. Management app + feature flag SDKs.
https://configcat.com/docs/sdk-reference/csharp
Other
29 stars 8 forks source link

Fixes subtle bug in the case of multiple Dispose calls #50

Closed adams85 closed 1 year ago

adams85 commented 1 year ago

Describe the purpose of your pull request

Consider the following pattern:

// 1. Creates a new instance of ConfigCatClient under the hood
var client1 = ConfigCatClient.Get("<KEY>");

// 2. Removes the previously created instance from the client cache
client1.Dispose();

// 3. Creates another instance of ConfigCatClient with the *same* key
var client2 = ConfigCatClient.Get("<KEY>");

// 4. Dispose can be called multiple times (it's not prohibited) but it should have no effect.
// However, if the Dispose logic just blindly removes the current cached instance by key,
// it will remove the other instance (client2), which would mess up the logic of shared instances.
// That is, we need to safeguard against this by making Dispose truly idempotent!
client1.Dispose();

A possible way of fixing this is presented in this PR. Another solution could be a disposed flag.

Requirement checklist (only if applicable)

adams85 commented 1 year ago

@z4kn4fein As far as I can see, the issue is also present in the Swift SDK.

z4kn4fein commented 1 year ago

@z4kn4fein As far as I can see, the issue is also present in the Swift SDK.

Here's the fix. WDYT?

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication