Skip to content

Commit d555d18

Browse files
Fix race condition causing NRE in TimerBatch
Fixed a race condition in BatchItemsCollection where concurrent calls to Complete() or Fail() on the same item could add null entries to failedItems, causing NullReferenceException when Remaining() calls Select(x => x.Item). Changes: - Added synchronization lock to protect critical sections in Complete/Fail - Changed failedItems from List<BatchItem<T>?> to List<BatchItem<T>> (non-nullable) - Protected FailedCount() and Remaining() with locks for thread-safe reads - Made Remaining() return materialized array to avoid deferred execution issues This ensures atomic check-and-set operations and prevents null entries in the failed items collection. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent bb9b427 commit d555d18

1 file changed

Lines changed: 34 additions & 13 deletions

File tree

AzureBatchQueue/TimerBatch.cs

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -208,40 +208,61 @@ TimeSpan CalculateFlushPeriod(TimeSpan visibilityTimeout)
208208
internal class BatchItemsCollection<T>
209209
{
210210
readonly BatchItem<T>?[] items;
211-
readonly List<BatchItem<T>?> failedItems;
211+
readonly List<BatchItem<T>> failedItems;
212+
readonly object syncLock = new();
212213
int notProcessedCount;
213214

214215
public BatchItemsCollection(BatchItem<T>[] items)
215216
{
216217
this.items = items;
217-
failedItems = new List<BatchItem<T>?>();
218+
failedItems = new List<BatchItem<T>>();
218219
notProcessedCount = this.items.Length;
219220
}
220221

221222
public int Complete(BatchItemId id)
222223
{
223-
if (items[id.Idx] == null)
224-
throw new ItemNotFoundException(id.ToString());
224+
lock (syncLock)
225+
{
226+
var item = items[id.Idx];
227+
if (item == null)
228+
throw new ItemNotFoundException(id.ToString());
225229

226-
items[id.Idx] = null;
227-
return Interlocked.Decrement(ref notProcessedCount);
230+
items[id.Idx] = null;
231+
return Interlocked.Decrement(ref notProcessedCount);
232+
}
228233
}
229234

230235
public int Fail(BatchItemId id)
231236
{
232-
if (items[id.Idx] == null)
233-
throw new ItemNotFoundException(id.ToString());
237+
lock (syncLock)
238+
{
239+
var item = items[id.Idx];
240+
if (item == null)
241+
throw new ItemNotFoundException(id.ToString());
234242

235-
failedItems.Add(items[id.Idx]);
236-
items[id.Idx] = null;
237-
return Interlocked.Decrement(ref notProcessedCount);
243+
failedItems.Add(item);
244+
items[id.Idx] = null;
245+
return Interlocked.Decrement(ref notProcessedCount);
246+
}
238247
}
239248

240249
public int NotProcessedCount() => notProcessedCount;
241-
public int FailedCount() => failedItems.Count;
250+
public int FailedCount()
251+
{
252+
lock (syncLock)
253+
{
254+
return failedItems.Count;
255+
}
256+
}
242257
public int RemainingCount() => notProcessedCount + FailedCount();
243258

244-
public IEnumerable<BatchItem<T>> Remaining() => failedItems.Concat(items.Where(x => x != null))!;
259+
public IEnumerable<BatchItem<T>> Remaining()
260+
{
261+
lock (syncLock)
262+
{
263+
return failedItems.Concat(items.Where(x => x != null)!).ToArray();
264+
}
265+
}
245266
public BatchItem<T>?[] Items() => items;
246267
}
247268

0 commit comments

Comments
 (0)