Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ private void WarmupWithdrawals(ParallelOptions parallelOptions, IReleaseSpec spe
});
}
}
catch (ObjectDisposedException)
{
// Ignore, disposed object pool policy (cancelled in test DI)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth logging?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be only in test context? Or maybe better we can expose something that will be awaited in tests, so this doesn't happen

}
catch (OperationCanceledException)
{
// Ignore, block completed cancel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,6 @@ public Block[] Process(BlockHeader? baseBlock, IReadOnlyList<Block> suggestedBlo
}

preBlockBaseBlock = processedBlock.Header;
// Make sure the prewarm task is finished before we reset the state
preWarmTask?.GetAwaiter().GetResult();
preWarmTask = null;
_stateProvider.Reset();

// Calculate the transaction hashes in the background and release tx sequence memory
Expand All @@ -190,7 +187,6 @@ public Block[] Process(BlockHeader? baseBlock, IReadOnlyList<Block> suggestedBlo
if (_logger.IsWarn) _logger.Warn($"Encountered exception {ex} while processing blocks.");
CancellationTokenExtensions.CancelDisposeAndClear(ref prewarmCancellation);
QueueClearCaches(preWarmTask);
preWarmTask?.GetAwaiter().GetResult();
RestoreBranch(previousBranchStateRoot);
throw;
}
Expand Down
67 changes: 52 additions & 15 deletions src/Nethermind/Nethermind.Core/Threading/ParallelUnbalancedWork.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: LGPL-3.0-only

using System;
using System.Runtime.ExceptionServices;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -20,6 +21,7 @@ public class ParallelUnbalancedWork : IThreadPoolWorkItem
};

private readonly Data _data;
private ExceptionDispatchInfo? _exception;

/// <summary>
/// Executes a parallel for loop over a range of integers.
Expand All @@ -43,19 +45,28 @@ public static void For(int fromInclusive, int toExclusive, ParallelOptions paral

Data data = new(threads, fromInclusive, toExclusive, action, parallelOptions.CancellationToken);

for (int i = 0; i < threads - 1; i++)
// Queue work items to the thread pool for all threads except the current one
var tasks = new ParallelUnbalancedWork[threads - 1];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ArrayPool?

for (int i = 0; i < tasks.Length; i++)
{
ThreadPool.UnsafeQueueUserWorkItem(new ParallelUnbalancedWork(data), preferLocal: false);
ThreadPool.UnsafeQueueUserWorkItem((tasks[i] = new ParallelUnbalancedWork(data)), preferLocal: false);
}

new ParallelUnbalancedWork(data).Execute();
var task = new ParallelUnbalancedWork(data);
task.Execute();

// If there are still active threads, wait for them to complete
if (data.ActiveThreads > 0)
{
data.Event.Wait();
}

task._exception?.Throw();
for (int i = 0; i < tasks.Length; i++)
{
tasks[i]._exception?.Throw();
}

parallelOptions.CancellationToken.ThrowIfCancellationRequested();
}

Expand Down Expand Up @@ -150,6 +161,13 @@ public void Execute()
i = _data.Index.GetNext();
}
}
catch (Exception ex)
{
if (ex is not OperationCanceledException)
{
_exception = ExceptionDispatchInfo.Capture(ex);
}
}
finally
{
// Signal that this thread has completed its work
Expand Down Expand Up @@ -237,6 +255,7 @@ private class Data(int threads, int fromInclusive, int toExclusive, Action<int>
private class InitProcessor<TLocal> : IThreadPoolWorkItem
{
private readonly Data<TLocal> _data;
private ExceptionDispatchInfo? _exception;

/// <summary>
/// Executes a parallel for loop over a range of integers, with thread-local data initialization and finalization.
Expand Down Expand Up @@ -266,20 +285,28 @@ public static void For(
var data = new Data<TLocal>(threads, fromInclusive, toExclusive, action, init, initValue, @finally, parallelOptions.CancellationToken);

// Queue work items to the thread pool for all threads except the current one
for (int i = 0; i < threads - 1; i++)
var tasks = new InitProcessor<TLocal>[threads - 1];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ArrayPool?

for (int i = 0; i < tasks.Length; i++)
{
ThreadPool.UnsafeQueueUserWorkItem(new InitProcessor<TLocal>(data), preferLocal: false);
ThreadPool.UnsafeQueueUserWorkItem((tasks[i] = new InitProcessor<TLocal>(data)), preferLocal: false);
}

// Execute work on the current thread
new InitProcessor<TLocal>(data).Execute();
var task = new InitProcessor<TLocal>(data);
task.Execute();

// If there are still active threads, wait for them to complete
if (data.ActiveThreads > 0)
{
data.Event.Wait();
}

task._exception?.Throw();
for (int i = 0; i < tasks.Length; i++)
{
tasks[i]._exception?.Throw();
}

parallelOptions.CancellationToken.ThrowIfCancellationRequested();
}

Expand All @@ -294,21 +321,31 @@ public static void For(
/// </summary>
public void Execute()
Comment thread
benaadams marked this conversation as resolved.
{
TLocal? value = _data.Init();
try
{
Comment thread
benaadams marked this conversation as resolved.
int i = _data.Index.GetNext();
while (i < _data.ToExclusive)
TLocal? value = _data.Init();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Always signal worker completion when initialization fails

If _data.Init() throws, control reaches the outer catch without executing the inner finally, so this worker never calls MarkThreadCompleted; a throwing _data.Finally has the same problem because the mark follows it. For then waits forever on data.Event, even with MaxDegreeOfParallelism = 1. The cache prewarmer's InitThreadState can throw while obtaining or building a read-only scope, which would hang cache cleanup, subsequent block processing, and shutdown. Put MarkThreadCompleted in an outer finally guaranteed to run and add initializer/finalizer exception tests.

try
{
if (_data.CancellationToken.IsCancellationRequested) return;
value = _data.Action(i, value);
i = _data.Index.GetNext();
int i = _data.Index.GetNext();
while (i < _data.ToExclusive)
{
if (_data.CancellationToken.IsCancellationRequested) return;
value = _data.Action(i, value);
i = _data.Index.GetNext();
}
}
finally
{
_data.Finally(value);
_data.MarkThreadCompleted();
}
}
finally
catch (Exception ex)
{
_data.Finally(value);
_data.MarkThreadCompleted();
if (ex is not OperationCanceledException)
{
_exception = ExceptionDispatchInfo.Capture(ex);
}
}
}

Expand Down