Skip to content

Commit 98ff404

Browse files
committed
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
1 parent cb666c1 commit 98ff404

3 files changed

Lines changed: 108 additions & 6 deletions

File tree

src/PostHog/Features/LocalFeatureFlagsLoader.cs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ internal sealed class LocalFeatureFlagsLoader(
2020
IOptions<PostHogOptions> options,
2121
ITaskScheduler taskScheduler,
2222
TimeProvider timeProvider,
23-
ILoggerFactory loggerFactory) : IDisposable
23+
ILoggerFactory loggerFactory) : IDisposable, IAsyncDisposable
2424
{
2525
volatile int _started;
26+
volatile int _disposed;
27+
Task? _pollingTask;
2628
LocalEvaluator? _localEvaluator;
2729
volatile string? _etag; // ETag for conditional requests to reduce bandwidth
2830
readonly CancellationTokenSource _cancellationTokenSource = new();
@@ -37,7 +39,7 @@ void StartPollingIfNotStarted()
3739
{
3840
return;
3941
}
40-
taskScheduler.Run(() => PollForFeatureFlagsAsync(_cancellationTokenSource.Token));
42+
_pollingTask = taskScheduler.Run(() => PollForFeatureFlagsAsync(_cancellationTokenSource.Token));
4143
}
4244

4345
/// <summary>
@@ -143,10 +145,22 @@ async Task PollForFeatureFlagsAsync(CancellationToken cancellationToken)
143145

144146
public bool IsLoaded => _localEvaluator is not null;
145147

146-
public void Dispose()
148+
public void Dispose() => DisposeAsync().AsTask().Wait();
149+
150+
public async ValueTask DisposeAsync()
147151
{
148-
_cancellationTokenSource.Dispose();
152+
if (Interlocked.Exchange(ref _disposed, 1) == 1)
153+
{
154+
return;
155+
}
156+
157+
// Cancel the token so the polling loop exits, then wait for it to finish.
158+
// This ensures any in-flight API call completes before we dispose resources.
159+
await _cancellationTokenSource.CancelAsync();
160+
await (_pollingTask ?? Task.CompletedTask);
161+
149162
_timer.Dispose();
163+
_cancellationTokenSource.Dispose();
150164
}
151165

152166
public void Clear()

src/PostHog/PostHogClient.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -691,11 +691,12 @@ public async Task LoadFeatureFlagsAsync(CancellationToken cancellationToken)
691691
/// <inheritdoc/>
692692
public async ValueTask DisposeAsync()
693693
{
694-
// Stop the polling and wait for it.
694+
// Stop background tasks first, while the API client is still alive.
695+
// The polling task in _featureFlagsLoader may call the API client during shutdown.
695696
await _asyncBatchHandler.DisposeAsync();
697+
await _featureFlagsLoader.DisposeAsync();
696698
_apiClient.Dispose();
697699
_featureFlagCalledEventCache.Dispose();
698-
_featureFlagsLoader.Dispose();
699700
}
700701

701702

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
using System.Net;
2+
using PostHog;
3+
using UnitTests.Fakes;
4+
#if NETCOREAPP3_1
5+
using TestLibrary.Fakes.Polyfills;
6+
#endif
7+
8+
namespace LocalFeatureFlagsLoaderTests;
9+
10+
public class TheDisposeAsyncMethod
11+
{
12+
const string LocalEvaluationResponse = """
13+
{
14+
"flags": [
15+
{
16+
"key": "test-flag",
17+
"active": true,
18+
"rollout_percentage": 100,
19+
"filters": {
20+
"groups": [
21+
{
22+
"properties": [],
23+
"rollout_percentage": 100
24+
}
25+
]
26+
}
27+
}
28+
]
29+
}
30+
""";
31+
32+
static readonly Uri LocalEvaluationUrl =
33+
new("https://us.i.posthog.com/api/feature_flag/local_evaluation?token=fake-project-api-key&send_cohorts");
34+
35+
[Fact]
36+
public async Task CompletesGracefullyDuringInFlightPoll()
37+
{
38+
var container = new TestContainer("fake-personal-api-key");
39+
var pollStarted = new TaskCompletionSource();
40+
var pollCanProceed = new TaskCompletionSource();
41+
42+
// First response succeeds immediately (the initial load).
43+
container.FakeHttpMessageHandler.AddLocalEvaluationResponse(LocalEvaluationResponse);
44+
45+
// Second response (the timer-triggered poll) blocks until we signal it.
46+
container.FakeHttpMessageHandler.AddResponse(
47+
LocalEvaluationUrl,
48+
HttpMethod.Get,
49+
async () =>
50+
{
51+
pollStarted.SetResult();
52+
await pollCanProceed.Task;
53+
return new HttpResponseMessage(HttpStatusCode.OK)
54+
{
55+
Content = new StringContent(
56+
LocalEvaluationResponse,
57+
System.Text.Encoding.UTF8,
58+
"application/json")
59+
};
60+
});
61+
62+
var client = container.Activate<PostHogClient>();
63+
64+
// Initial load starts the polling loop and makes the first API call.
65+
await client.LoadFeatureFlagsAsync(CancellationToken.None);
66+
67+
// Advance past the poll interval so the background poll fires.
68+
container.FakeTimeProvider.Advance(TimeSpan.FromSeconds(31));
69+
70+
// Wait for the poll's API call to begin.
71+
await pollStarted.Task;
72+
73+
// Begin disposal while the poll is mid-flight.
74+
var disposeTask = client.DisposeAsync().AsTask();
75+
76+
// Unblock the in-flight API call so the poll can finish.
77+
pollCanProceed.SetResult();
78+
79+
// Verify disposal completes without deadlock or exception.
80+
var timeout = TimeSpan.FromSeconds(5);
81+
var completed = await Task.WhenAny(disposeTask, Task.Delay(timeout));
82+
if (completed != disposeTask)
83+
{
84+
throw new TimeoutException("DisposeAsync did not complete within 5 seconds; possible deadlock.");
85+
}
86+
}
87+
}

0 commit comments

Comments
 (0)