Skip to content

Commit

Permalink
Fix NullReferenceException in ObjectPool when reusing all returne…
Browse files Browse the repository at this point in the history
…d items (#904)

* Replaced delegate with Action<T>

* Change delegate to Action<T>

* Add unit test to confirm issue

* Fix NullReferenceException in ObjectPool when reusing all returned items

The ObjectPool's Use() method was throwing a NullReferenceException when
all items had been returned to the pool and New() was called again. This
was due to _tailNode being null in this scenario.

Added a null check for _tailNode in Use() method to properly reinitialize
the linked list structure when the pool has been emptied and refilled.
  • Loading branch information
AristurtleDev authored Jun 29, 2024
1 parent 9347374 commit 1f3f1c5
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 8 deletions.
10 changes: 5 additions & 5 deletions source/MonoGame.Extended/Collections/IPoolable.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
namespace MonoGame.Extended.Collections
{
public delegate void ReturnToPoolDelegate(IPoolable poolable);
using System;

namespace MonoGame.Extended.Collections
{
public interface IPoolable
{
IPoolable NextNode { get; set; }
IPoolable PreviousNode { get; set; }
void Initialize(ReturnToPoolDelegate returnDelegate);
void Initialize(Action<IPoolable> returnDelegate);
void Return();
}
}
}
12 changes: 9 additions & 3 deletions source/MonoGame.Extended/Collections/ObjectPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public enum ObjectPoolIsFullPolicy
public class ObjectPool<T> : IEnumerable<T>
where T : class, IPoolable
{
private readonly ReturnToPoolDelegate _returnToPoolDelegate;
private readonly Action<IPoolable> _returnToPoolDelegate;

private readonly Deque<T> _freeItems; // circular buffer for O(1) operations
private T _headNode; // linked list for iteration
Expand Down Expand Up @@ -144,7 +144,13 @@ private void Use(T item)
{
item.Initialize(_returnToPoolDelegate);
item.NextNode = null;
if (item != _tailNode)
if(_tailNode is null)
{
_headNode = item;
_tailNode = item;
item.PreviousNode = null;
}
else
{
item.PreviousNode = _tailNode;
_tailNode.NextNode = item;
Expand All @@ -154,4 +160,4 @@ private void Use(T item)
ItemUsed?.Invoke(item);
}
}
}
}
51 changes: 51 additions & 0 deletions tests/MonoGame.Extended.Tests/Collections/ObjectPoolTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright (c) Craftwork Games. All rights reserved.
// Licensed under the MIT license.
// See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using MonoGame.Extended.Collections;

namespace MonoGame.Extended.Tests.Collections;

public class ObjectPoolTests
{
private class TestPoolable : IPoolable
{
public Action<IPoolable> ReturnAction { get; private set; }
public IPoolable NextNode { get; set; }
public IPoolable PreviousNode { get; set; }

public void Initialize(Action<IPoolable> returnAction)
{
ReturnAction = returnAction;
}

public void Return()
{
ReturnAction(this);
}
}

[Fact]
public void ObjectPool_ThrowsNullReferenceException_WhenAllItemsReturnedAndNewCalled()
{
// Arrange
var pool = new ObjectPool<TestPoolable>(() => new TestPoolable(), 2);

// Act & Assert
var item1 = pool.New();
var item2 = pool.New();

// Return all items to the pool
item1.Return();
item2.Return();


var exception = Record.Exception(() => pool.New());
Assert.Null(exception);
}
}

0 comments on commit 1f3f1c5

Please sign in to comment.