From 98ff404db0e2a8e9ba73b4210be41498f7a74d37 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Wed, 25 Feb 2026 17:38:02 -0800 Subject: [PATCH 1/4] Fix fire-and-forget background task in LocalFeatureFlagsLoader Await the polling task during disposal so in-flight API calls complete before resources are disposed. Follows the same pattern established in AsyncBatchHandler (PR #158). Fixes #159 --- .../Features/LocalFeatureFlagsLoader.cs | 22 ++++- src/PostHog/PostHogClient.cs | 5 +- .../Features/LocalFeatureFlagsLoaderTests.cs | 87 +++++++++++++++++++ 3 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs diff --git a/src/PostHog/Features/LocalFeatureFlagsLoader.cs b/src/PostHog/Features/LocalFeatureFlagsLoader.cs index 75689354..a89b2161 100644 --- a/src/PostHog/Features/LocalFeatureFlagsLoader.cs +++ b/src/PostHog/Features/LocalFeatureFlagsLoader.cs @@ -20,9 +20,11 @@ internal sealed class LocalFeatureFlagsLoader( IOptions options, ITaskScheduler taskScheduler, TimeProvider timeProvider, - ILoggerFactory loggerFactory) : IDisposable + ILoggerFactory loggerFactory) : IDisposable, IAsyncDisposable { volatile int _started; + volatile int _disposed; + Task? _pollingTask; LocalEvaluator? _localEvaluator; volatile string? _etag; // ETag for conditional requests to reduce bandwidth readonly CancellationTokenSource _cancellationTokenSource = new(); @@ -37,7 +39,7 @@ void StartPollingIfNotStarted() { return; } - taskScheduler.Run(() => PollForFeatureFlagsAsync(_cancellationTokenSource.Token)); + _pollingTask = taskScheduler.Run(() => PollForFeatureFlagsAsync(_cancellationTokenSource.Token)); } /// @@ -143,10 +145,22 @@ async Task PollForFeatureFlagsAsync(CancellationToken cancellationToken) public bool IsLoaded => _localEvaluator is not null; - public void Dispose() + public void Dispose() => DisposeAsync().AsTask().Wait(); + + public async ValueTask DisposeAsync() { - _cancellationTokenSource.Dispose(); + if (Interlocked.Exchange(ref _disposed, 1) == 1) + { + return; + } + + // Cancel the token so the polling loop exits, then wait for it to finish. + // This ensures any in-flight API call completes before we dispose resources. + await _cancellationTokenSource.CancelAsync(); + await (_pollingTask ?? Task.CompletedTask); + _timer.Dispose(); + _cancellationTokenSource.Dispose(); } public void Clear() diff --git a/src/PostHog/PostHogClient.cs b/src/PostHog/PostHogClient.cs index 789e496a..a4f670a8 100644 --- a/src/PostHog/PostHogClient.cs +++ b/src/PostHog/PostHogClient.cs @@ -691,11 +691,12 @@ public async Task LoadFeatureFlagsAsync(CancellationToken cancellationToken) /// public async ValueTask DisposeAsync() { - // Stop the polling and wait for it. + // Stop background tasks first, while the API client is still alive. + // The polling task in _featureFlagsLoader may call the API client during shutdown. await _asyncBatchHandler.DisposeAsync(); + await _featureFlagsLoader.DisposeAsync(); _apiClient.Dispose(); _featureFlagCalledEventCache.Dispose(); - _featureFlagsLoader.Dispose(); } diff --git a/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs b/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs new file mode 100644 index 00000000..7a92c553 --- /dev/null +++ b/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs @@ -0,0 +1,87 @@ +using System.Net; +using PostHog; +using UnitTests.Fakes; +#if NETCOREAPP3_1 +using TestLibrary.Fakes.Polyfills; +#endif + +namespace LocalFeatureFlagsLoaderTests; + +public class TheDisposeAsyncMethod +{ + const string LocalEvaluationResponse = """ + { + "flags": [ + { + "key": "test-flag", + "active": true, + "rollout_percentage": 100, + "filters": { + "groups": [ + { + "properties": [], + "rollout_percentage": 100 + } + ] + } + } + ] + } + """; + + static readonly Uri LocalEvaluationUrl = + new("https://us.i.posthog.com/api/feature_flag/local_evaluation?token=fake-project-api-key&send_cohorts"); + + [Fact] + public async Task CompletesGracefullyDuringInFlightPoll() + { + var container = new TestContainer("fake-personal-api-key"); + var pollStarted = new TaskCompletionSource(); + var pollCanProceed = new TaskCompletionSource(); + + // First response succeeds immediately (the initial load). + container.FakeHttpMessageHandler.AddLocalEvaluationResponse(LocalEvaluationResponse); + + // Second response (the timer-triggered poll) blocks until we signal it. + container.FakeHttpMessageHandler.AddResponse( + LocalEvaluationUrl, + HttpMethod.Get, + async () => + { + pollStarted.SetResult(); + await pollCanProceed.Task; + return new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent( + LocalEvaluationResponse, + System.Text.Encoding.UTF8, + "application/json") + }; + }); + + var client = container.Activate(); + + // Initial load starts the polling loop and makes the first API call. + await client.LoadFeatureFlagsAsync(CancellationToken.None); + + // Advance past the poll interval so the background poll fires. + container.FakeTimeProvider.Advance(TimeSpan.FromSeconds(31)); + + // Wait for the poll's API call to begin. + await pollStarted.Task; + + // Begin disposal while the poll is mid-flight. + var disposeTask = client.DisposeAsync().AsTask(); + + // Unblock the in-flight API call so the poll can finish. + pollCanProceed.SetResult(); + + // Verify disposal completes without deadlock or exception. + var timeout = TimeSpan.FromSeconds(5); + var completed = await Task.WhenAny(disposeTask, Task.Delay(timeout)); + if (completed != disposeTask) + { + throw new TimeoutException("DisposeAsync did not complete within 5 seconds; possible deadlock."); + } + } +} From 3f8af573ca11e5c8dc31a9ec1832b10f32a147f9 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Wed, 25 Feb 2026 17:51:47 -0800 Subject: [PATCH 2/4] Add disposal tests and mark _pollingTask volatile Add three tests for LocalFeatureFlagsLoader disposal: - DoesNotDisposeTwice: concurrent double-dispose completes without exception - CompletesGracefullyWhenPollingNeverStarted: dispose before any polling - Await disposeTask after timeout check to surface disposal exceptions Mark _pollingTask volatile for consistency with other cross-thread fields and to make the intent explicit, since unlike AsyncBatchHandler's readonly task fields, this one is lazily assigned after construction. --- .../Features/LocalFeatureFlagsLoader.cs | 2 +- .../Features/LocalFeatureFlagsLoaderTests.cs | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/PostHog/Features/LocalFeatureFlagsLoader.cs b/src/PostHog/Features/LocalFeatureFlagsLoader.cs index a89b2161..641dbab6 100644 --- a/src/PostHog/Features/LocalFeatureFlagsLoader.cs +++ b/src/PostHog/Features/LocalFeatureFlagsLoader.cs @@ -24,7 +24,7 @@ internal sealed class LocalFeatureFlagsLoader( { volatile int _started; volatile int _disposed; - Task? _pollingTask; + volatile Task? _pollingTask; LocalEvaluator? _localEvaluator; volatile string? _etag; // ETag for conditional requests to reduce bandwidth readonly CancellationTokenSource _cancellationTokenSource = new(); diff --git a/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs b/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs index 7a92c553..b04c53ad 100644 --- a/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs +++ b/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs @@ -83,5 +83,32 @@ public async Task CompletesGracefullyDuringInFlightPoll() { throw new TimeoutException("DisposeAsync did not complete within 5 seconds; possible deadlock."); } + + // Surface any exception thrown during disposal. + await disposeTask; + } + + [Fact] + public async Task DoesNotDisposeTwice() + { + var container = new TestContainer("fake-personal-api-key"); + container.FakeHttpMessageHandler.AddLocalEvaluationResponse(LocalEvaluationResponse); + + var client = container.Activate(); + await client.LoadFeatureFlagsAsync(CancellationToken.None); + + await Task.WhenAll( + client.DisposeAsync().AsTask(), + client.DisposeAsync().AsTask()); + } + + [Fact] + public async Task CompletesGracefullyWhenPollingNeverStarted() + { + var container = new TestContainer(); + var client = container.Activate(); + + // Dispose without ever calling LoadFeatureFlagsAsync. + await client.DisposeAsync(); } } From 4085324f4430d374d017c5d4c4ca166f4af3c613 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Wed, 25 Feb 2026 18:30:41 -0800 Subject: [PATCH 3/4] Use GetAwaiter().GetResult() instead of .Wait() in sync Dispose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoids wrapping exceptions in AggregateException, giving clearer stack traces from disposal paths. Applied consistently across all three sync-over-async Dispose bridges. Also fix misleading comment in LocalFeatureFlagsLoader — cancellation terminates the in-flight request, it doesn't wait for it to complete. --- src/PostHog/Features/LocalFeatureFlagsLoader.cs | 6 +++--- src/PostHog/Library/AsyncBatchHandler.cs | 2 +- src/PostHog/PostHogClient.cs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/PostHog/Features/LocalFeatureFlagsLoader.cs b/src/PostHog/Features/LocalFeatureFlagsLoader.cs index 641dbab6..1b3e73d9 100644 --- a/src/PostHog/Features/LocalFeatureFlagsLoader.cs +++ b/src/PostHog/Features/LocalFeatureFlagsLoader.cs @@ -145,7 +145,7 @@ async Task PollForFeatureFlagsAsync(CancellationToken cancellationToken) public bool IsLoaded => _localEvaluator is not null; - public void Dispose() => DisposeAsync().AsTask().Wait(); + public void Dispose() => DisposeAsync().AsTask().GetAwaiter().GetResult(); public async ValueTask DisposeAsync() { @@ -154,8 +154,8 @@ public async ValueTask DisposeAsync() return; } - // Cancel the token so the polling loop exits, then wait for it to finish. - // This ensures any in-flight API call completes before we dispose resources. + // Cancel the token so the polling loop exits, then wait for it to finish + // (either by completing normally or via cancellation) before disposing resources. await _cancellationTokenSource.CancelAsync(); await (_pollingTask ?? Task.CompletedTask); diff --git a/src/PostHog/Library/AsyncBatchHandler.cs b/src/PostHog/Library/AsyncBatchHandler.cs index 419b356a..c364923a 100644 --- a/src/PostHog/Library/AsyncBatchHandler.cs +++ b/src/PostHog/Library/AsyncBatchHandler.cs @@ -233,7 +233,7 @@ async Task SendBatch(IReadOnlyCollection batch) public void Dispose() { - DisposeAsync().AsTask().Wait(); + DisposeAsync().AsTask().GetAwaiter().GetResult(); } public async ValueTask DisposeAsync() diff --git a/src/PostHog/PostHogClient.cs b/src/PostHog/PostHogClient.cs index a4f670a8..3ef81fc8 100644 --- a/src/PostHog/PostHogClient.cs +++ b/src/PostHog/PostHogClient.cs @@ -681,7 +681,7 @@ public async Task LoadFeatureFlagsAsync(CancellationToken cancellationToken) } /// - public void Dispose() => DisposeAsync().AsTask().Wait(); + public void Dispose() => DisposeAsync().AsTask().GetAwaiter().GetResult(); /// /// Clears the local flags cache. From 855ce2a3940f42e7e2ddfee455437e17194a304b Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Wed, 25 Feb 2026 18:39:42 -0800 Subject: [PATCH 4/4] Wrap async disposal in try/finally to guarantee resource cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensures timers, CTS, and API clients are always disposed even if awaiting a background task throws. The exception still propagates — this just prevents resource leaks on top of bugs. Applied consistently to LocalFeatureFlagsLoader, AsyncBatchHandler, and PostHogClient. --- .../Features/LocalFeatureFlagsLoader.cs | 15 ++++++++++----- src/PostHog/Library/AsyncBatchHandler.cs | 19 ++++++++++++------- src/PostHog/PostHogClient.cs | 14 ++++++++++---- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/PostHog/Features/LocalFeatureFlagsLoader.cs b/src/PostHog/Features/LocalFeatureFlagsLoader.cs index 1b3e73d9..06c92f12 100644 --- a/src/PostHog/Features/LocalFeatureFlagsLoader.cs +++ b/src/PostHog/Features/LocalFeatureFlagsLoader.cs @@ -156,11 +156,16 @@ public async ValueTask DisposeAsync() // Cancel the token so the polling loop exits, then wait for it to finish // (either by completing normally or via cancellation) before disposing resources. - await _cancellationTokenSource.CancelAsync(); - await (_pollingTask ?? Task.CompletedTask); - - _timer.Dispose(); - _cancellationTokenSource.Dispose(); + try + { + await _cancellationTokenSource.CancelAsync(); + await (_pollingTask ?? Task.CompletedTask); + } + finally + { + _timer.Dispose(); + _cancellationTokenSource.Dispose(); + } } public void Clear() diff --git a/src/PostHog/Library/AsyncBatchHandler.cs b/src/PostHog/Library/AsyncBatchHandler.cs index c364923a..fdf90f1f 100644 --- a/src/PostHog/Library/AsyncBatchHandler.cs +++ b/src/PostHog/Library/AsyncBatchHandler.cs @@ -249,13 +249,18 @@ public async ValueTask DisposeAsync() // Cancel the token so both background loops exit, then wait for them to finish. // This ensures any in-flight flush completes and _flushing returns to 0 before // we attempt the final flush below. - await _cancellationTokenSource.CancelAsync(); - await Task.WhenAll(_timerTask, _flushSignalTask); - - _timer.Dispose(); - _flushSignal.Dispose(); - _cancellationTokenSource.Dispose(); - _channel.Writer.Complete(); + try + { + await _cancellationTokenSource.CancelAsync(); + await Task.WhenAll(_timerTask, _flushSignalTask); + } + finally + { + _timer.Dispose(); + _flushSignal.Dispose(); + _cancellationTokenSource.Dispose(); + _channel.Writer.Complete(); + } try { _logger.LogTraceFlushCalledInDispose(Count); diff --git a/src/PostHog/PostHogClient.cs b/src/PostHog/PostHogClient.cs index 3ef81fc8..53686fb6 100644 --- a/src/PostHog/PostHogClient.cs +++ b/src/PostHog/PostHogClient.cs @@ -693,10 +693,16 @@ public async ValueTask DisposeAsync() { // Stop background tasks first, while the API client is still alive. // The polling task in _featureFlagsLoader may call the API client during shutdown. - await _asyncBatchHandler.DisposeAsync(); - await _featureFlagsLoader.DisposeAsync(); - _apiClient.Dispose(); - _featureFlagCalledEventCache.Dispose(); + try + { + await _asyncBatchHandler.DisposeAsync(); + await _featureFlagsLoader.DisposeAsync(); + } + finally + { + _apiClient.Dispose(); + _featureFlagCalledEventCache.Dispose(); + } }