Skip to content

Commit dea2d97

Browse files
fix(audience): drop DiskStore._cachedCount, read file count from disk
Removes the cached on-disk file counter and the BumpCount delta tracking that maintained it. Count() now calls Directory.GetFiles each time. Why - File.Delete silently succeeds when the path is missing, so TryDelete returned true regardless of whether a real file went away. Two concurrent flushes that both called Delete on the same paths each got true back and each called BumpCount(-1), drifting the cached count negative. - Shutdown's force-clear of _sendInFlight followed by an ungated final SendBatchAsync gave the race a real opening (visible as the StatusBar_QueueSizeIncrementsAfterTrack flake on Linux Unity 6, where finalCount went past zero into "got -2"). - The cached counter was already documented as approximate ("Tests that plant files outside the DiskStore API will drift this and should assert on filesystem state, not Count()"), so callers could not rely on it anyway. Trade-off: Count() is now O(N) on the queue directory. The only caller is ImmutableAudience.QueueSize, read from a 500 ms UI poll and a couple of test assertions, so this is fine for any realistic queue size and removes an entire class of drift. TryDelete drops its bool return because nothing needs to know whether a real file was removed once we are not counting deltas.
1 parent 809e141 commit dea2d97

1 file changed

Lines changed: 21 additions & 29 deletions

File tree

src/Packages/Audience/Runtime/Transport/DiskStore.cs

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.Diagnostics.CodeAnalysis;
66
using System.IO;
77
using System.Linq;
8-
using System.Threading;
98

109
namespace Immutable.Audience
1110
{
@@ -15,17 +14,10 @@ internal sealed class DiskStore
1514
{
1615
private readonly string _queueDir;
1716

18-
// Cached queue file count: on-disk count at construction, plus
19-
// tracked deltas from Write/Delete. Tests that plant files
20-
// outside the DiskStore API will drift this and should assert
21-
// on filesystem state, not Count().
22-
private int _cachedCount;
23-
2417
internal DiskStore(string persistentDataPath)
2518
{
2619
_queueDir = Path.Combine(persistentDataPath, "imtbl_audience", "queue");
2720
Directory.CreateDirectory(_queueDir);
28-
_cachedCount = Directory.GetFiles(_queueDir, "*.json").Length;
2921
}
3022

3123
// Atomically writes json as a new event file.
@@ -37,7 +29,6 @@ internal void Write(string json)
3729

3830
File.WriteAllText(tmpPath, json);
3931

40-
var replaced = false;
4132
try
4233
{
4334
File.Move(tmpPath, finalPath);
@@ -46,10 +37,7 @@ internal void Write(string json)
4637
{
4738
File.Delete(finalPath);
4839
File.Move(tmpPath, finalPath);
49-
replaced = true;
5040
}
51-
52-
if (!replaced) BumpCount(+1);
5341
}
5442

5543
// Returns up to maxSize file paths, oldest first. Stale files
@@ -86,7 +74,7 @@ internal IReadOnlyList<string> ReadBatch(int maxSize)
8674
var fileTime = new DateTime(ticks, DateTimeKind.Utc);
8775
if (fileTime < cutoff)
8876
{
89-
if (TryDelete(path)) BumpCount(-1);
77+
TryDelete(path);
9078
continue;
9179
}
9280
}
@@ -101,30 +89,34 @@ internal IReadOnlyList<string> ReadBatch(int maxSize)
10189
internal void Delete(IEnumerable<string> paths)
10290
{
10391
foreach (var path in paths)
104-
if (TryDelete(path)) BumpCount(-1);
92+
TryDelete(path);
10593
}
10694

107-
// Total number of event files currently on disk. Reads the cached
108-
// count seeded at construction; mutating ops maintain it.
109-
internal int Count() => Volatile.Read(ref _cachedCount);
110-
111-
private void BumpCount(int delta) => Interlocked.Add(ref _cachedCount, delta);
95+
// Total number of event files currently on disk. Reads the filesystem
96+
// each call so concurrent Write / Delete from any thread is reflected
97+
// without a separately maintained counter that could drift.
98+
internal int Count()
99+
{
100+
try { return Directory.GetFiles(_queueDir, "*.json").Length; }
101+
catch (DirectoryNotFoundException) { return 0; }
102+
}
112103

113-
private static bool TryDelete(string path)
104+
private static void TryDelete(string path)
114105
{
115-
try { File.Delete(path); return true; }
116-
catch (DirectoryNotFoundException) { return true; }
117-
catch (IOException) { return false; }
118-
catch (UnauthorizedAccessException) { return false; }
106+
try { File.Delete(path); }
107+
catch (DirectoryNotFoundException) { }
108+
catch (IOException) { }
109+
catch (UnauthorizedAccessException) { }
119110
}
111+
120112
internal void DeleteAll()
121113
{
122114
string[] paths;
123115
try { paths = Directory.GetFiles(_queueDir, "*.json"); }
124116
catch (DirectoryNotFoundException) { return; }
125117

126118
foreach (var path in paths)
127-
if (TryDelete(path)) BumpCount(-1);
119+
TryDelete(path);
128120
}
129121

130122
// Drops queued identify/alias files, strips userId from track files.
@@ -145,13 +137,13 @@ private void ApplyAnonymousDowngradeToFile(string path)
145137
!msg.TryGetValue(MessageFields.Type, out var typeObj) ||
146138
!(typeObj is string type))
147139
{
148-
if (TryDelete(path)) BumpCount(-1);
140+
TryDelete(path);
149141
return;
150142
}
151143

152144
if (IsIdentityMessage(type))
153145
{
154-
if (TryDelete(path)) BumpCount(-1);
146+
TryDelete(path);
155147
return;
156148
}
157149

@@ -195,11 +187,11 @@ private void RewriteTrackWithoutUserId(string path, Dictionary<string, object> m
195187
catch (IOException)
196188
{
197189
// Delete rather than leave the old userId-bearing payload.
198-
if (TryDelete(path)) BumpCount(-1);
190+
TryDelete(path);
199191
}
200192
catch (UnauthorizedAccessException)
201193
{
202-
if (TryDelete(path)) BumpCount(-1);
194+
TryDelete(path);
203195
}
204196
}
205197

0 commit comments

Comments
 (0)