From c175cd73017a0c52da9468db0ba0aa8781eb0fc8 Mon Sep 17 00:00:00 2001 From: Ledjon Behluli Date: Sun, 4 May 2025 00:24:42 +0200 Subject: [PATCH] avoid "collection was modified" while canceling grain operations --- src/Orleans.Runtime/Catalog/ActivationData.cs | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/Orleans.Runtime/Catalog/ActivationData.cs b/src/Orleans.Runtime/Catalog/ActivationData.cs index 4b8f47c76c7..32e8af53a18 100644 --- a/src/Orleans.Runtime/Catalog/ActivationData.cs +++ b/src/Orleans.Runtime/Catalog/ActivationData.cs @@ -1,5 +1,6 @@ #nullable enable using System; +using System.Buffers; using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -452,28 +453,46 @@ private void CancelPendingOperations() lock (this) { // If the grain is currently activating, cancel that operation. - if (_pendingOperations is not { } operations) + if (_pendingOperations is not { Count: > 0 } operations) { return; } - foreach (var op in operations) + var opCount = operations.Count; + // We could be using an ArrayPool here, but we would need to filter out the + // non-command types, which means we would need to loop over _pendingOperations, which + // defeats the purpose of making a snapshot. + // Note: CopyTo does not use the queue's enumerator, so its safe to take a snapshot this way. + var array = ArrayPool.Shared.Rent(opCount); + + operations.CopyTo(array, 0); + + var snapshot = new Span(array, 0, opCount); + + try { - if (op is Command cmd) + foreach (var op in snapshot) { - try - { - cmd.Cancel(); - } - catch (Exception exception) + if (op is Command cmd) { - if (exception is not ObjectDisposedException) + try { - LogErrorCancellingOperation(_shared.Logger, exception, cmd); + cmd.Cancel(); + } + catch (Exception exception) + { + if (exception is not ObjectDisposedException) + { + LogErrorCancellingOperation(_shared.Logger, exception, cmd); + } } } } } + finally + { + ArrayPool.Shared.Return(array, true); + } } }