Fix dispose block.AccountChanges ArrayPoolList

[Marchhill]

Changes

• Dispose ArrayPoolList

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

Remarks

Fixes the following error observed on chiado node:

Unhandled exception. System.InvalidOperationException: ArrayPoolList hasn't been disposed. Created    at Nethermind.Core.Collections.ArrayPoolList`1..ctor(ArrayPool`1 arrayPool, Int32 capacity, Int32 startingCount)
   at Nethermind.Core.Collections.ArrayPoolList`1..ctor(Int32 capacity)
   at Nethermind.State.StateProvider.ChangedAddresses()
   at Nethermind.State.WorldState.Nethermind.State.IWorldState.GetAccountChanges()
   at Nethermind.State.WorldStateMetricsDecorator.GetAccountChanges()
   at Nethermind.Consensus.Processing.BlockProcessor.ProcessBlock(Block block, IBlockTracer blockTracer, ProcessingOptions options)
   at Nethermind.Consensus.AuRa.AuRaBlockProcessor.PostMergeProcessBlock(Block block, IBlockTracer blockTracer, ProcessingOptions options)
   at Nethermind.Merge.AuRa.AuRaMergeBlockProcessor.ProcessBlock(Block block, IBlockTracer blockTracer, ProcessingOptions options)
   at Nethermind.Consensus.Processing.BlockProcessor.ProcessOne(Block suggestedBlock, ProcessingOptions options, IBlockTracer blockTracer)
   at Nethermind.Consensus.Processing.BlockProcessor.Process(Hash256 newBranchStateRoot, List`1 suggestedBlocks, ProcessingOptions options, IBlockTracer blockTracer)
   at Nethermind.Consensus.Processing.BlockchainProcessor.ProcessBranch(ProcessingBranch& processingBranch, ProcessingOptions options, IBlockTracer tracer, String& error)
   at Nethermind.Consensus.Processing.BlockchainProcessor.Process(Block suggestedBlock, ProcessingOptions options, IBlockTracer tracer, String& error)
   at Nethermind.Consensus.Processing.BlockchainProcessor.RunProcessingLoop()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext(Thread threadPoolThread)
   at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1.SetResult(TResult result)
   at System.Threading.Channels.ChannelReader`1.ReadAllAsync(CancellationToken cancellationToken)+MoveNext()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext(Thread threadPoolThread)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()

   at Nethermind.Core.Collections.ArrayPoolList`1.Finalize() in /root/nethermind/src/Nethermind/Nethermind.Core/Collections/ArrayPoolList.cs:line 386
   at System.GC.RunFinalizers()

[benaadams]

Is Disposed here in TxPool; so this would mean it was disposed before the txPool could use it

nethermind/src/Nethermind/Nethermind.TxPool/TxPool.cs

Lines 226 to 238 in b0c5165

	ArrayPoolList<AddressAsKey>? accountChanges = args.Block.AccountChanges;
	if (!CanUseCache(args.Block, accountChanges))
	{
	// Not sequential block, reset cache
	_accountCache.Reset();
	}
	else
	{
	// Sequential block, just remove changed accounts from cache
	_accountCache.RemoveAccounts(accountChanges);
	}
	args.Block.AccountChanges = null;
	accountChanges?.Dispose();

However; OnHeadChange isn't always fired to the txpool for every block 🤔

[MarekM25]

Could you try to write unit tests that reproduce it? Of course, if possible

[Marchhill]

Is Disposed here in TxPool; so this would mean it was disposed before the txPool could use it

nethermind/src/Nethermind/Nethermind.TxPool/TxPool.cs

Lines 226 to 238 in b0c5165

	ArrayPoolList<AddressAsKey>? accountChanges = args.Block.AccountChanges;
	if (!CanUseCache(args.Block, accountChanges))
	{
	// Not sequential block, reset cache
	_accountCache.Reset();
	}
	else
	{
	// Sequential block, just remove changed accounts from cache
	_accountCache.RemoveAccounts(accountChanges);
	}
	args.Block.AccountChanges = null;
	accountChanges?.Dispose();

However; OnHeadChange isn't always fired to the txpool for every block 🤔

I think this the TxPool would use it before my code disposes it. I added just after the Process function.

The call stack would look something like this Process -> _blockTree.UpdateMainChain -> MoveToMain -> invoke BlockReplacementEventArgs -> TxPool ProcessNewHeads

So the change should just dispose if this is never called

[Marchhill]

Could you try to write unit tests that reproduce it? Of course, if possible

I'll have a look into it, happens for me just from starting the node on chiado

[LukaszRozmej]

@benaadams ?

[LukaszRozmej]

I think this is used in TxPool so cannot be disposed here

[Marchhill]

see my above comment, I think that's before

[LukaszRozmej]

Is Disposed here in TxPool; so this would mean it was disposed before the txPool could use it

nethermind/src/Nethermind/Nethermind.TxPool/TxPool.cs

Lines 226 to 238 in b0c5165

	ArrayPoolList<AddressAsKey>? accountChanges = args.Block.AccountChanges;
	if (!CanUseCache(args.Block, accountChanges))
	{
	// Not sequential block, reset cache
	_accountCache.Reset();
	}
	else
	{
	// Sequential block, just remove changed accounts from cache
	_accountCache.RemoveAccounts(accountChanges);
	}
	args.Block.AccountChanges = null;
	accountChanges?.Dispose();

However; OnHeadChange isn't always fired to the txpool for every block 🤔

BlockAddedToMain is fired on every block

[LukaszRozmej]

Is Disposed here in TxPool; so this would mean it was disposed before the txPool could use it

nethermind/src/Nethermind/Nethermind.TxPool/TxPool.cs

Lines 226 to 238 in b0c5165

	ArrayPoolList<AddressAsKey>? accountChanges = args.Block.AccountChanges;
	if (!CanUseCache(args.Block, accountChanges))
	{
	// Not sequential block, reset cache
	_accountCache.Reset();
	}
	else
	{
	// Sequential block, just remove changed accounts from cache
	_accountCache.RemoveAccounts(accountChanges);
	}
	args.Block.AccountChanges = null;
	accountChanges?.Dispose();

However; OnHeadChange isn't always fired to the txpool for every block 🤔

I think this the TxPool would use it before my code disposes it. I added just after the Process function.

The call stack would look something like this Process -> _blockTree.UpdateMainChain -> MoveToMain -> invoke BlockReplacementEventArgs -> TxPool ProcessNewHeads

So the change should just dispose if this is never called

Can you add a test for it? :)