Skip to content

Commit a790f78

Browse files
fix: More robust shutdown/cleanup/reset (#188)
Signed-off-by: Austin Drenski <[email protected]>
1 parent 1a14f6c commit a790f78

File tree

7 files changed

+69
-94
lines changed

7 files changed

+69
-94
lines changed

build/Common.props

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<Project>
22
<PropertyGroup>
3-
<LangVersion>7.3</LangVersion>
3+
<LangVersion>latest</LangVersion>
44
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
55
<EnableNETAnalyzers>true</EnableNETAnalyzers>
66
</PropertyGroup>
@@ -19,6 +19,7 @@
1919
Please sort alphabetically.
2020
Refer to https://docs.microsoft.com/nuget/concepts/package-versioning for semver syntax.
2121
-->
22+
<MicrosoftBclAsyncInterfacesVer>[8.0.0,)</MicrosoftBclAsyncInterfacesVer>
2223
<MicrosoftExtensionsLoggerVer>[2.0,)</MicrosoftExtensionsLoggerVer>
2324
<MicrosoftSourceLinkGitHubPkgVer>[1.0.0,2.0)</MicrosoftSourceLinkGitHubPkgVer>
2425
</PropertyGroup>

src/OpenFeature/Api.cs

+23-11
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,13 @@ namespace OpenFeature
1717
public sealed class Api : IEventBus
1818
{
1919
private EvaluationContext _evaluationContext = EvaluationContext.Empty;
20-
private readonly ProviderRepository _repository = new ProviderRepository();
20+
private EventExecutor _eventExecutor = new EventExecutor();
21+
private ProviderRepository _repository = new ProviderRepository();
2122
private readonly ConcurrentStack<Hook> _hooks = new ConcurrentStack<Hook>();
2223

2324
/// The reader/writer locks are not disposed because the singleton instance should never be disposed.
2425
private readonly ReaderWriterLockSlim _evaluationContextLock = new ReaderWriterLockSlim();
2526

26-
internal readonly EventExecutor EventExecutor = new EventExecutor();
27-
28-
2927
/// <summary>
3028
/// Singleton instance of Api
3129
/// </summary>
@@ -45,7 +43,7 @@ private Api() { }
4543
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
4644
public async Task SetProvider(FeatureProvider featureProvider)
4745
{
48-
this.EventExecutor.RegisterDefaultFeatureProvider(featureProvider);
46+
this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider);
4947
await this._repository.SetProvider(featureProvider, this.GetContext()).ConfigureAwait(false);
5048
}
5149

@@ -58,7 +56,7 @@ public async Task SetProvider(FeatureProvider featureProvider)
5856
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
5957
public async Task SetProvider(string clientName, FeatureProvider featureProvider)
6058
{
61-
this.EventExecutor.RegisterClientFeatureProvider(clientName, featureProvider);
59+
this._eventExecutor.RegisterClientFeatureProvider(clientName, featureProvider);
6260
await this._repository.SetProvider(clientName, featureProvider, this.GetContext()).ConfigureAwait(false);
6361
}
6462

@@ -224,20 +222,28 @@ public EvaluationContext GetContext()
224222
/// </summary>
225223
public async Task Shutdown()
226224
{
227-
await this._repository.Shutdown().ConfigureAwait(false);
228-
await this.EventExecutor.Shutdown().ConfigureAwait(false);
225+
await using (this._eventExecutor.ConfigureAwait(false))
226+
await using (this._repository.ConfigureAwait(false))
227+
{
228+
this._evaluationContext = EvaluationContext.Empty;
229+
this._hooks.Clear();
230+
231+
// TODO: make these lazy to avoid extra allocations on the common cleanup path?
232+
this._eventExecutor = new EventExecutor();
233+
this._repository = new ProviderRepository();
234+
}
229235
}
230236

231237
/// <inheritdoc />
232238
public void AddHandler(ProviderEventTypes type, EventHandlerDelegate handler)
233239
{
234-
this.EventExecutor.AddApiLevelHandler(type, handler);
240+
this._eventExecutor.AddApiLevelHandler(type, handler);
235241
}
236242

237243
/// <inheritdoc />
238244
public void RemoveHandler(ProviderEventTypes type, EventHandlerDelegate handler)
239245
{
240-
this.EventExecutor.RemoveApiLevelHandler(type, handler);
246+
this._eventExecutor.RemoveApiLevelHandler(type, handler);
241247
}
242248

243249
/// <summary>
@@ -246,7 +252,13 @@ public void RemoveHandler(ProviderEventTypes type, EventHandlerDelegate handler)
246252
/// <param name="logger">The logger to be used</param>
247253
public void SetLogger(ILogger logger)
248254
{
249-
this.EventExecutor.Logger = logger;
255+
this._eventExecutor.Logger = logger;
250256
}
257+
258+
internal void AddClientHandler(string client, ProviderEventTypes eventType, EventHandlerDelegate handler)
259+
=> this._eventExecutor.AddClientHandler(client, eventType, handler);
260+
261+
internal void RemoveClientHandler(string client, ProviderEventTypes eventType, EventHandlerDelegate handler)
262+
=> this._eventExecutor.RemoveClientHandler(client, eventType, handler);
251263
}
252264
}

src/OpenFeature/EventExecutor.cs

+31-71
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,13 @@
1010

1111
namespace OpenFeature
1212
{
13-
14-
internal delegate Task ShutdownDelegate();
15-
16-
internal class EventExecutor
13+
internal class EventExecutor : IAsyncDisposable
1714
{
1815
private readonly object _lockObj = new object();
1916
public readonly Channel<object> EventChannel = Channel.CreateBounded<object>(1);
20-
private FeatureProviderReference _defaultProvider;
21-
private readonly Dictionary<string, FeatureProviderReference> _namedProviderReferences = new Dictionary<string, FeatureProviderReference>();
22-
private readonly List<FeatureProviderReference> _activeSubscriptions = new List<FeatureProviderReference>();
23-
private readonly SemaphoreSlim _shutdownSemaphore = new SemaphoreSlim(0);
24-
25-
private ShutdownDelegate _shutdownDelegate;
17+
private FeatureProvider _defaultProvider;
18+
private readonly Dictionary<string, FeatureProvider> _namedProviderReferences = new Dictionary<string, FeatureProvider>();
19+
private readonly List<FeatureProvider> _activeSubscriptions = new List<FeatureProvider>();
2620

2721
private readonly Dictionary<ProviderEventTypes, List<EventHandlerDelegate>> _apiHandlers = new Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>();
2822
private readonly Dictionary<string, Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>> _clientHandlers = new Dictionary<string, Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>>();
@@ -32,11 +26,12 @@ internal class EventExecutor
3226
public EventExecutor()
3327
{
3428
this.Logger = new Logger<EventExecutor>(new NullLoggerFactory());
35-
this._shutdownDelegate = this.SignalShutdownAsync;
3629
var eventProcessing = new Thread(this.ProcessEventAsync);
3730
eventProcessing.Start();
3831
}
3932

33+
public ValueTask DisposeAsync() => new(this.Shutdown());
34+
4035
internal void AddApiLevelHandler(ProviderEventTypes eventType, EventHandlerDelegate handler)
4136
{
4237
lock (this._lockObj)
@@ -114,7 +109,7 @@ internal void RegisterDefaultFeatureProvider(FeatureProvider provider)
114109
{
115110
var oldProvider = this._defaultProvider;
116111

117-
this._defaultProvider = new FeatureProviderReference(provider);
112+
this._defaultProvider = provider;
118113

119114
this.StartListeningAndShutdownOld(this._defaultProvider, oldProvider);
120115
}
@@ -128,8 +123,8 @@ internal void RegisterClientFeatureProvider(string client, FeatureProvider provi
128123
}
129124
lock (this._lockObj)
130125
{
131-
var newProvider = new FeatureProviderReference(provider);
132-
FeatureProviderReference oldProvider = null;
126+
var newProvider = provider;
127+
FeatureProvider oldProvider = null;
133128
if (this._namedProviderReferences.TryGetValue(client, out var foundOldProvider))
134129
{
135130
oldProvider = foundOldProvider;
@@ -141,7 +136,7 @@ internal void RegisterClientFeatureProvider(string client, FeatureProvider provi
141136
}
142137
}
143138

144-
private void StartListeningAndShutdownOld(FeatureProviderReference newProvider, FeatureProviderReference oldProvider)
139+
private void StartListeningAndShutdownOld(FeatureProvider newProvider, FeatureProvider oldProvider)
145140
{
146141
// check if the provider is already active - if not, we need to start listening for its emitted events
147142
if (!this.IsProviderActive(newProvider))
@@ -154,15 +149,11 @@ private void StartListeningAndShutdownOld(FeatureProviderReference newProvider,
154149
if (oldProvider != null && !this.IsProviderBound(oldProvider))
155150
{
156151
this._activeSubscriptions.Remove(oldProvider);
157-
var channel = oldProvider.Provider.GetEventChannel();
158-
if (channel != null)
159-
{
160-
channel.Writer.WriteAsync(new ShutdownSignal());
161-
}
152+
oldProvider.GetEventChannel()?.Writer.Complete();
162153
}
163154
}
164155

165-
private bool IsProviderBound(FeatureProviderReference provider)
156+
private bool IsProviderBound(FeatureProvider provider)
166157
{
167158
if (this._defaultProvider == provider)
168159
{
@@ -178,18 +169,18 @@ private bool IsProviderBound(FeatureProviderReference provider)
178169
return false;
179170
}
180171

181-
private bool IsProviderActive(FeatureProviderReference providerRef)
172+
private bool IsProviderActive(FeatureProvider providerRef)
182173
{
183174
return this._activeSubscriptions.Contains(providerRef);
184175
}
185176

186-
private void EmitOnRegistration(FeatureProviderReference provider, ProviderEventTypes eventType, EventHandlerDelegate handler)
177+
private void EmitOnRegistration(FeatureProvider provider, ProviderEventTypes eventType, EventHandlerDelegate handler)
187178
{
188179
if (provider == null)
189180
{
190181
return;
191182
}
192-
var status = provider.Provider.GetStatus();
183+
var status = provider.GetStatus();
193184

194185
var message = "";
195186
if (status == ProviderStatus.Ready && eventType == ProviderEventTypes.ProviderReady)
@@ -211,7 +202,7 @@ private void EmitOnRegistration(FeatureProviderReference provider, ProviderEvent
211202
{
212203
handler.Invoke(new ProviderEventPayload
213204
{
214-
ProviderName = provider.Provider?.GetMetadata()?.Name,
205+
ProviderName = provider.GetMetadata()?.Name,
215206
Type = eventType,
216207
Message = message
217208
});
@@ -225,33 +216,33 @@ private void EmitOnRegistration(FeatureProviderReference provider, ProviderEvent
225216

226217
private async void ProcessFeatureProviderEventsAsync(object providerRef)
227218
{
228-
while (true)
219+
var typedProviderRef = (FeatureProvider)providerRef;
220+
if (typedProviderRef.GetEventChannel() is not { Reader: { } reader })
229221
{
230-
var typedProviderRef = (FeatureProviderReference)providerRef;
231-
if (typedProviderRef.Provider.GetEventChannel() == null)
232-
{
233-
return;
234-
}
235-
var item = await typedProviderRef.Provider.GetEventChannel().Reader.ReadAsync().ConfigureAwait(false);
222+
return;
223+
}
224+
225+
while (await reader.WaitToReadAsync().ConfigureAwait(false))
226+
{
227+
if (!reader.TryRead(out var item))
228+
continue;
236229

237230
switch (item)
238231
{
239232
case ProviderEventPayload eventPayload:
240233
await this.EventChannel.Writer.WriteAsync(new Event { Provider = typedProviderRef, EventPayload = eventPayload }).ConfigureAwait(false);
241234
break;
242-
case ShutdownSignal _:
243-
typedProviderRef.ShutdownSemaphore.Release();
244-
return;
245235
}
246236
}
247237
}
248238

249239
// Method to process events
250240
private async void ProcessEventAsync()
251241
{
252-
while (true)
242+
while (await this.EventChannel.Reader.WaitToReadAsync().ConfigureAwait(false))
253243
{
254-
var item = await this.EventChannel.Reader.ReadAsync().ConfigureAwait(false);
244+
if (!this.EventChannel.Reader.TryRead(out var item))
245+
continue;
255246

256247
switch (item)
257248
{
@@ -307,9 +298,6 @@ private async void ProcessEventAsync()
307298
}
308299
}
309300
break;
310-
case ShutdownSignal _:
311-
this._shutdownSemaphore.Release();
312-
return;
313301
}
314302

315303
}
@@ -329,43 +317,15 @@ private void InvokeEventHandler(EventHandlerDelegate eventHandler, Event e)
329317

330318
public async Task Shutdown()
331319
{
332-
await this._shutdownDelegate().ConfigureAwait(false);
333-
}
320+
this.EventChannel.Writer.Complete();
334321

335-
internal void SetShutdownDelegate(ShutdownDelegate del)
336-
{
337-
this._shutdownDelegate = del;
338-
}
339-
340-
// Method to signal shutdown
341-
private async Task SignalShutdownAsync()
342-
{
343-
// Enqueue a shutdown signal
344-
await this.EventChannel.Writer.WriteAsync(new ShutdownSignal()).ConfigureAwait(false);
345-
346-
// Wait for the processing loop to acknowledge the shutdown
347-
await this._shutdownSemaphore.WaitAsync().ConfigureAwait(false);
348-
}
349-
}
350-
351-
internal class ShutdownSignal
352-
{
353-
}
354-
355-
internal class FeatureProviderReference
356-
{
357-
internal readonly SemaphoreSlim ShutdownSemaphore = new SemaphoreSlim(0);
358-
internal FeatureProvider Provider { get; }
359-
360-
public FeatureProviderReference(FeatureProvider provider)
361-
{
362-
this.Provider = provider;
322+
await this.EventChannel.Reader.Completion.ConfigureAwait(false);
363323
}
364324
}
365325

366326
internal class Event
367327
{
368-
internal FeatureProviderReference Provider { get; set; }
328+
internal FeatureProvider Provider { get; set; }
369329
internal ProviderEventPayload EventPayload { get; set; }
370330
}
371331
}

src/OpenFeature/OpenFeature.csproj

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
</PropertyGroup>
88

99
<ItemGroup>
10+
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="$(MicrosoftBclAsyncInterfacesVer)" Condition="'$(TargetFramework)' == 'net462'" />
11+
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="$(MicrosoftBclAsyncInterfacesVer)" Condition="'$(TargetFramework)' == 'netstandard2.0'" />
1012
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="$(MicrosoftExtensionsLoggerVer)" />
1113
</ItemGroup>
1214

src/OpenFeature/OpenFeatureClient.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,13 @@ public FeatureClient(string name, string version, ILogger logger = null, Evaluat
9696
/// <inheritdoc />
9797
public void AddHandler(ProviderEventTypes eventType, EventHandlerDelegate handler)
9898
{
99-
Api.Instance.EventExecutor.AddClientHandler(this._metadata.Name, eventType, handler);
99+
Api.Instance.AddClientHandler(this._metadata.Name, eventType, handler);
100100
}
101101

102102
/// <inheritdoc />
103103
public void RemoveHandler(ProviderEventTypes type, EventHandlerDelegate handler)
104104
{
105-
Api.Instance.EventExecutor.RemoveClientHandler(this._metadata.Name, type, handler);
105+
Api.Instance.RemoveClientHandler(this._metadata.Name, type, handler);
106106
}
107107

108108
/// <inheritdoc />

src/OpenFeature/ProviderRepository.cs

+9-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ namespace OpenFeature
1212
/// <summary>
1313
/// This class manages the collection of providers, both default and named, contained by the API.
1414
/// </summary>
15-
internal sealed class ProviderRepository
15+
internal sealed class ProviderRepository : IAsyncDisposable
1616
{
1717
private FeatureProvider _defaultProvider = new NoOpFeatureProvider();
1818

@@ -31,6 +31,14 @@ internal sealed class ProviderRepository
3131
/// of that provider under different names..
3232
private readonly ReaderWriterLockSlim _providersLock = new ReaderWriterLockSlim();
3333

34+
public async ValueTask DisposeAsync()
35+
{
36+
using (this._providersLock)
37+
{
38+
await this.Shutdown().ConfigureAwait(false);
39+
}
40+
}
41+
3442
/// <summary>
3543
/// Set the default provider
3644
/// </summary>

test/OpenFeature.Tests/OpenFeatureTests.cs

-8
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,6 @@ namespace OpenFeature.Tests
1111
{
1212
public class OpenFeatureTests : ClearOpenFeatureInstanceFixture
1313
{
14-
static async Task EmptyShutdown()
15-
{
16-
await Task.FromResult(0).ConfigureAwait(false);
17-
}
18-
1914
[Fact]
2015
[Specification("1.1.1", "The `API`, and any state it maintains SHOULD exist as a global singleton, even in cases wherein multiple versions of the `API` are present at runtime.")]
2116
public void OpenFeature_Should_Be_Singleton()
@@ -79,9 +74,6 @@ public async Task OpenFeature_Should_Shutdown_Unused_Provider()
7974
[Specification("1.6.1", "The API MUST define a mechanism to propagate a shutdown request to active providers.")]
8075
public async Task OpenFeature_Should_Support_Shutdown()
8176
{
82-
// configure the shutdown method of the event executor to do nothing
83-
// to prevent eventing tests from failing
84-
Api.Instance.EventExecutor.SetShutdownDelegate(EmptyShutdown);
8577
var providerA = Substitute.For<FeatureProvider>();
8678
providerA.GetStatus().Returns(ProviderStatus.NotReady);
8779

0 commit comments

Comments
 (0)