Merge pull request #6136 from peppy/dpguard

Guard against potential usage of `DrawablePool` without being added to hierarchy

osu.Framework.Tests/Visual/Drawables/TestSceneDrawablePool.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using System;
 using System.Collections.Generic;
 using System.Linq;
@@ -22,8 +20,8 @@ namespace osu.Framework.Tests.Visual.Drawables
 {
     public partial class TestSceneDrawablePool : TestScene
     {
-        private DrawablePool<TestDrawable> pool;
-        private SpriteText count;
+        private DrawablePool<TestDrawable> pool = null!;
+        private SpriteText? count;
 
         private readonly HashSet<TestDrawable> consumed = new HashSet<TestDrawable>();
 
@@ -85,7 +83,7 @@ public void TestReturnWithoutAdding()
         {
             resetWithNewPool(() => new TestPool(TimePerAction, 1));
 
-            TestDrawable drawable = null;
+            TestDrawable drawable = null!;
 
             AddStep("consume without adding", () => drawable = pool.Get());
 
@@ -113,7 +111,7 @@ public void TestPoolReturnWhenAboveCapacity()
         {
             resetWithNewPool(() => new TestPool(TimePerAction * 20, 1, 1));
 
-            TestDrawable first = null, second = null;
+            TestDrawable first = null!, second = null!;
 
             AddStep("consume item", () => first = consumeDrawable());
 
@@ -142,8 +140,8 @@ public void TestPrepareAndFreeMethods()
         {
             resetWithNewPool(() => new TestPool(TimePerAction, 1));
 
-            TestDrawable drawable = null;
-            TestDrawable drawable2 = null;
+            TestDrawable drawable = null!;
+            TestDrawable drawable2 = null!;
 
             AddStep("consume item", () => drawable = consumeDrawable());
 
@@ -165,8 +163,8 @@ public void TestPrepareOnlyOnceOnMultipleUsages()
         {
             resetWithNewPool(() => new TestPool(TimePerAction, 1));
 
-            TestDrawable drawable = null;
-            TestDrawable drawable2 = null;
+            TestDrawable drawable = null!;
+            TestDrawable drawable2 = null!;
 
             AddStep("consume item", () => drawable = consumeDrawable(false));
 
@@ -189,7 +187,7 @@ public void TestPrepareOnlyOnceOnMultipleUsages()
         [Test]
         public void TestUsePoolableDrawableWithoutPool()
         {
-            TestDrawable drawable = null;
+            TestDrawable drawable = null!;
 
             AddStep("consume item", () => Add(drawable = new TestDrawable()));
 
@@ -266,7 +264,11 @@ public void TestPoolUsageExceedsMaximum(int maxPoolSize)
         [Test]
         public void TestGetFromNotLoadedPool()
         {
-            Assert.DoesNotThrow(() => new TestPool(100, 1).Get());
+            // could theoretically be made to work - and did previously work -
+            // but it would work in a counterintuitive manner
+            // (as it will not preload the initial count of drawables in advance on load thread),
+            // so to avoid a footgun make this throw.
+            Assert.Throws<InvalidOperationException>(() => new TestPool(100, 1).Get());
         }
 
         /// <summary>
@@ -278,7 +280,7 @@ public void TestParentInvalidationFromChildDoesNotReturnPooledParent()
         {
             resetWithNewPool(() => new TestPool(TimePerAction, 1));
 
-            TestDrawable drawable = null;
+            TestDrawable drawable = null!;
 
             AddStep("consume item", () => drawable = consumeDrawable(false));
             AddStep("add child", () => drawable.AddChild(Empty()));
@@ -290,7 +292,7 @@ public void TestDrawablePreparedWhenClockRewound()
         {
             resetWithNewPool(() => new TestPool(TimePerAction, 1));
 
-            TestDrawable drawable = null;
+            TestDrawable drawable = null!;
 
             AddStep("consume item and rewind clock", () =>
             {

osu.Framework/Graphics/Pooling/DrawablePool.cs

@@ -111,6 +111,9 @@ public void Return(PoolableDrawable pooledDrawable)
         /// <returns>The drawable.</returns>
         public T Get(Action<T> setupAction = null)
         {
+            if (LoadState <= LoadState.Loading)
+                throw new InvalidOperationException($"A {nameof(DrawablePool<T>)} must be in a loaded state before retrieving pooled drawables.");
+
             if (!pool.TryPop(out var drawable))
             {
                 drawable = create();