Skip to content

Commit

Permalink
fixup: remove ct for init/shutdown
Browse files Browse the repository at this point in the history 
Signed-off-by: Todd Baert <[email protected]>
  • Loading branch information
@toddbaert
toddbaert committed May 3, 2024
1 parent 7cf37fb commit 027078d
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 40 deletions.
29 changes: 14 additions & 15 deletions src/OpenFeature/Api.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,10 @@ private Api() { }
/// </summary>
/// <remarks>The provider cannot be set to null. Attempting to set the provider to null has no effect.</remarks>
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to cancel any async side effects.</param>
public async Task SetProviderAsync(FeatureProvider featureProvider, CancellationToken cancellationToken = default)
public async Task SetProviderAsync(FeatureProvider featureProvider)
{
this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider);
await this._repository.SetProviderAsync(featureProvider, this.GetContext(), cancellationToken: cancellationToken).ConfigureAwait(false);
await this._repository.SetProviderAsync(featureProvider, this.GetContext()).ConfigureAwait(false);
}


Expand All @@ -56,15 +55,14 @@ public async Task SetProviderAsync(FeatureProvider featureProvider, Cancellation
/// </summary>
/// <param name="clientName">Name of client</param>
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to cancel any async side effects.</param>
public async Task SetProviderAsync(string clientName, FeatureProvider featureProvider, CancellationToken cancellationToken = default)
public async Task SetProviderAsync(string clientName, FeatureProvider featureProvider)
{
if (string.IsNullOrWhiteSpace(clientName))
{
throw new ArgumentNullException(nameof(clientName));
}
this._eventExecutor.RegisterClientFeatureProvider(clientName, featureProvider);
await this._repository.SetProviderAsync(clientName, featureProvider, this.GetContext(), cancellationToken: cancellationToken).ConfigureAwait(false);
await this._repository.SetProviderAsync(clientName, featureProvider, this.GetContext()).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -227,17 +225,18 @@ public EvaluationContext GetContext()
/// Once shut down is complete, API is reset and ready to use again.
/// </para>
/// </summary>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to cancel any async side effects.</param>
public async Task ShutdownAsync(CancellationToken cancellationToken = default)
public async Task ShutdownAsync()
{
await this._repository.ShutdownAsync(cancellationToken: cancellationToken).ConfigureAwait(false);
await this._eventExecutor.ShutdownAsync(cancellationToken).ConfigureAwait(false);
this._evaluationContext = EvaluationContext.Empty;
this._hooks.Clear();
await using (this._eventExecutor.ConfigureAwait(false))
await using (this._repository.ConfigureAwait(false))
{
this._evaluationContext = EvaluationContext.Empty;
this._hooks.Clear();

// TODO: make these lazy to avoid extra allocations on the common cleanup path?
this._eventExecutor = new EventExecutor();
this._repository = new ProviderRepository();
// TODO: make these lazy to avoid extra allocations on the common cleanup path?
this._eventExecutor = new EventExecutor();
this._repository = new ProviderRepository();
}
}

/// <inheritdoc />
Expand Down
6 changes: 4 additions & 2 deletions src/OpenFeature/EventExecutor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace OpenFeature
{
internal delegate Task ShutdownDelegate(CancellationToken cancellationToken);

internal sealed partial class EventExecutor
internal sealed partial class EventExecutor : IAsyncDisposable
{
private readonly object _lockObj = new object();
public readonly Channel<object> EventChannel = Channel.CreateBounded<object>(1);
Expand All @@ -32,6 +32,8 @@ public EventExecutor()
eventProcessing.Start();
}

public ValueTask DisposeAsync() => new(this.ShutdownAsync());

internal void SetLogger(ILogger logger) => this._logger = logger;

internal void AddApiLevelHandler(ProviderEventTypes eventType, EventHandlerDelegate handler)
Expand Down Expand Up @@ -317,7 +319,7 @@ private void InvokeEventHandler(EventHandlerDelegate eventHandler, Event e)
}
}

public async Task ShutdownAsync(CancellationToken cancellationToken = default)
public async Task ShutdownAsync()
{
this.EventChannel.Writer.Complete();
await this.EventChannel.Reader.Completion.ConfigureAwait(false);
Expand Down
41 changes: 18 additions & 23 deletions src/OpenFeature/ProviderRepository.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,13 @@ public async ValueTask DisposeAsync()
/// initialization
/// </param>
/// <param name="afterShutdown">called after a provider is shutdown, can be used to remove event handlers</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to cancel any async side effects.</param>
public async ValueTask SetProviderAsync(
public async Task SetProviderAsync(
FeatureProvider? featureProvider,
EvaluationContext context,
Action<FeatureProvider>? afterSet = null,
Action<FeatureProvider>? afterInitialization = null,
Action<FeatureProvider, Exception>? afterError = null,
Action<FeatureProvider>? afterShutdown = null,
CancellationToken cancellationToken = default)
Action<FeatureProvider>? afterShutdown = null)
{
// Cannot unset the feature provider.
if (featureProvider == null)
Expand All @@ -94,24 +92,23 @@ public async ValueTask SetProviderAsync(
// We want to allow shutdown to happen concurrently with initialization, and the caller to not
// wait for it.
#pragma warning disable CS4014
this.ShutdownIfUnusedAsync(oldProvider, afterShutdown, afterError, cancellationToken);
this.ShutdownIfUnusedAsync(oldProvider, afterShutdown, afterError);
#pragma warning restore CS4014
}
finally
{
this._providersLock.ExitWriteLock();
}

await InitProviderAsync(this._defaultProvider, context, afterInitialization, afterError, cancellationToken)
await InitProviderAsync(this._defaultProvider, context, afterInitialization, afterError)
.ConfigureAwait(false);
}

private static async ValueTask InitProviderAsync(
private static async Task InitProviderAsync(
FeatureProvider? newProvider,
EvaluationContext context,
Action<FeatureProvider>? afterInitialization,
Action<FeatureProvider, Exception>? afterError,
CancellationToken cancellationToken)
Action<FeatureProvider, Exception>? afterError)
{
if (newProvider == null)
{
Expand All @@ -121,7 +118,7 @@ private static async ValueTask InitProviderAsync(
{
try
{
await newProvider.InitializeAsync(context, cancellationToken).ConfigureAwait(false);
await newProvider.InitializeAsync(context).ConfigureAwait(false);
afterInitialization?.Invoke(newProvider);
}
catch (Exception ex)
Expand Down Expand Up @@ -156,7 +153,7 @@ private static async ValueTask InitProviderAsync(
/// </param>
/// <param name="afterShutdown">called after a provider is shutdown, can be used to remove event handlers</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to cancel any async side effects.</param>
public async ValueTask SetProviderAsync(string clientName,
public async Task SetProviderAsync(string clientName,
FeatureProvider? featureProvider,
EvaluationContext context,
Action<FeatureProvider>? afterSet = null,
Expand Down Expand Up @@ -192,25 +189,24 @@ public async ValueTask SetProviderAsync(string clientName,
// We want to allow shutdown to happen concurrently with initialization, and the caller to not
// wait for it.
#pragma warning disable CS4014
this.ShutdownIfUnusedAsync(oldProvider, afterShutdown, afterError, cancellationToken);
this.ShutdownIfUnusedAsync(oldProvider, afterShutdown, afterError);
#pragma warning restore CS4014
}
finally
{
this._providersLock.ExitWriteLock();
}

await InitProviderAsync(featureProvider, context, afterInitialization, afterError, cancellationToken).ConfigureAwait(false);
await InitProviderAsync(featureProvider, context, afterInitialization, afterError).ConfigureAwait(false);
}

/// <remarks>
/// Shutdown the feature provider if it is unused. This must be called within a write lock of the _providersLock.
/// </remarks>
private async ValueTask ShutdownIfUnusedAsync(
private async Task ShutdownIfUnusedAsync(
FeatureProvider? targetProvider,
Action<FeatureProvider>? afterShutdown,
Action<FeatureProvider, Exception>? afterError,
CancellationToken cancellationToken)
Action<FeatureProvider, Exception>? afterError)
{
if (ReferenceEquals(this._defaultProvider, targetProvider))
{
Expand All @@ -222,7 +218,7 @@ private async ValueTask ShutdownIfUnusedAsync(
return;
}

await SafeShutdownProviderAsync(targetProvider, afterShutdown, afterError, cancellationToken).ConfigureAwait(false);
await SafeShutdownProviderAsync(targetProvider, afterShutdown, afterError).ConfigureAwait(false);
}

/// <remarks>
Expand All @@ -234,10 +230,9 @@ private async ValueTask ShutdownIfUnusedAsync(
/// it would not be meaningful to emit an error.
/// </para>
/// </remarks>
private static async ValueTask SafeShutdownProviderAsync(FeatureProvider? targetProvider,
private static async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider,
Action<FeatureProvider>? afterShutdown,
Action<FeatureProvider, Exception>? afterError,
CancellationToken cancellationToken)
Action<FeatureProvider, Exception>? afterError)
{
if (targetProvider == null)
{
Expand All @@ -246,7 +241,7 @@ private static async ValueTask SafeShutdownProviderAsync(FeatureProvider? target

try
{
await targetProvider.ShutdownAsync(cancellationToken).ConfigureAwait(false);
await targetProvider.ShutdownAsync().ConfigureAwait(false);
afterShutdown?.Invoke(targetProvider);
}
catch (Exception ex)
Expand Down Expand Up @@ -288,7 +283,7 @@ public FeatureProvider GetProvider(string? clientName)
: this.GetProvider();
}

public async ValueTask ShutdownAsync(Action<FeatureProvider, Exception>? afterError = null, CancellationToken cancellationToken = default)
public async Task ShutdownAsync(Action<FeatureProvider, Exception>? afterError = null, CancellationToken cancellationToken = default)
{
var providers = new HashSet<FeatureProvider>();
this._providersLock.EnterWriteLock();
Expand All @@ -312,7 +307,7 @@ public async ValueTask ShutdownAsync(Action<FeatureProvider, Exception>? afterEr
foreach (var targetProvider in providers)
{
// We don't need to take any actions after shutdown.
await SafeShutdownProviderAsync(targetProvider, null, afterError, cancellationToken).ConfigureAwait(false);
await SafeShutdownProviderAsync(targetProvider, null, afterError).ConfigureAwait(false);
}
}
}
Expand Down

0 comments on commit 027078d

Please sign in to comment.