Fix broken critical section in static World create/destroy

src/Arch.Tests/WorldConcurrencyTest.cs

@@ -0,0 +1,135 @@
+// The static World registry (World.Worlds / World.Create bookkeeping) does not exist
+// under PURE_ECS, and neither do the Entity extension methods used below.
+#if !PURE_ECS
+using Arch.Core;
+using Arch.Core.Extensions;
+using static NUnit.Framework.Assert;
+
+namespace Arch.Tests;
+
+/// <summary>
+///     The <see cref="WorldConcurrencyTest"/> class
+///     tests concurrent <see cref="World.Create()"/>/<see cref="World.Destroy"/> against the
+///     static <see cref="World.Worlds"/> storage.
+///
+///     Historically <c>World.Create</c> locked the <see cref="World.Worlds"/> array itself and
+///     replaced that array on resize, so after a resize concurrent creators locked different
+///     objects and raced: duplicate world ids, lost slot writes (a created world resolving to
+///     <c>null</c> through <c>World.Worlds[entity.WorldId]</c>) and cross-wired entity storage.
+/// </summary>
+[TestFixture]
+public sealed class WorldConcurrencyTest
+{
+    /// <summary>
+    ///     Concurrently creates worlds (forcing several <see cref="World.Worlds"/> resizes),
+    ///     uses each created world immediately and checks id uniqueness.
+    /// </summary>
+    [Test]
+    public void ConcurrentWorldCreateProducesUniqueUsableWorlds()
+    {
+        const int threads = 8;
+        const int worldsPerThread = 64;
+
+        var created = new World[threads * worldsPerThread];
+        using var barrier = new Barrier(threads);
+
+        RunOnThreads(threads, threadIndex =>
+        {
+            barrier.SignalAndWait();
+            for (var i = 0; i < worldsPerThread; i++)
+            {
+                var world = World.Create();
+                created[(threadIndex * worldsPerThread) + i] = world;
+
+                // Use the world through the static lookup right away — this is the read path
+                // (EntityExtensions/generated accessors) that observed null slots pre-fix.
+                var entity = world.Create(new Transform { X = threadIndex, Y = i });
+                That(entity.IsAlive(), Is.True);
+                That(entity.Get<Transform>().X, Is.EqualTo(threadIndex));
+            }
+        });
+
+        try
+        {
+            var ids = new HashSet<int>();
+            foreach (var world in created)
+            {
+                That(world, Is.Not.Null);
+                That(ids.Add(world.Id), Is.True, $"Duplicate world id {world.Id} handed out concurrently.");
+                That(World.Worlds[world.Id], Is.SameAs(world));
+            }
+        }
+        finally
+        {
+            foreach (var world in created)
+            {
+                if (world != null)
+                {
+                    World.Destroy(world);
+                }
+            }
+        }
+    }
+
+    /// <summary>
+    ///     Churns concurrent create → use → destroy cycles so id recycling, slot writes and
+    ///     resizes interleave across threads.
+    /// </summary>
+    [Test]
+    public void ConcurrentWorldCreateDestroyChurnDoesNotCorruptStaticStorage()
+    {
+        const int threads = 8;
+        const int rounds = 200;
+
+        using var barrier = new Barrier(threads);
+
+        RunOnThreads(threads, threadIndex =>
+        {
+            barrier.SignalAndWait();
+            for (var round = 0; round < rounds; round++)
+            {
+                var world = World.Create();
+                var entity = world.Create(new Transform { X = round, Y = threadIndex });
+                That(entity.Get<Transform>().Y, Is.EqualTo(threadIndex));
+                That(World.Worlds[world.Id], Is.SameAs(world));
+                World.Destroy(world);
+            }
+        });
+    }
+
+    private static void RunOnThreads(int threadCount, Action<int> body)
+    {
+        var failures = new List<Exception>();
+        var workers = new Thread[threadCount];
+        for (var t = 0; t < threadCount; t++)
+        {
+            var threadIndex = t;
+            workers[t] = new Thread(() =>
+            {
+                try
+                {
+                    body(threadIndex);
+                }
+                catch (Exception exception)
+                {
+                    lock (failures)
+                    {
+                        failures.Add(exception);
+                    }
+                }
+            });
+            workers[t].Start();
+        }
+
+        foreach (var worker in workers)
+        {
+            worker.Join();
+        }
+
+        if (failures.Count > 0)
+        {
+            throw new AggregateException(failures);
+        }
+    }
+}
+#endif

src/Arch.Tests/WorldTest.cs

@@ -48,11 +48,31 @@ public void Teardown()
     [Test]
     public void WorldRecycle()
     {
-        var firstWorld = World.Create();
-        World.Destroy(firstWorld);
+        // Recycled ids are handed out FIFO, so drain the queue left behind by earlier
+        // world-churning tests to make the reuse check below deterministic. The queue can
+        // never hold more ids than the id space allocated so far, which Worlds.Length bounds.
+        var drained = new World[World.Worlds.Length];
+        for (var index = 0; index < drained.Length; index++)
+        {
+            drained[index] = World.Create();
+        }
 
-        var secondWorld = World.Create();
-        That(secondWorld.Id, Is.EqualTo(firstWorld.Id));
+        try
+        {
+            var firstWorld = World.Create();
+            World.Destroy(firstWorld);
+
+            var secondWorld = World.Create();
+            That(secondWorld.Id, Is.EqualTo(firstWorld.Id));
+            World.Destroy(secondWorld);
+        }
+        finally
+        {
+            foreach (var world in drained)
+            {
+                World.Destroy(world);
+            }
+        }
     }
 
     /// <summary>

src/Arch/Core/World.cs

@@ -72,7 +72,22 @@ public partial class World
     ///     A list of all existing <see cref="Worlds"/>.
     ///     Should not be modified by the user.
     /// </summary>
-    public static World[] Worlds { get; private set; } = new World[4];
+    public static World[] Worlds
+    {
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        get => _worlds;
+    }
+
+    private static World[] _worlds = new World[4];
+
+    /// <summary>
+    ///     Guards <see cref="_worlds"/>, <see cref="RecycledWorldIds"/> and world id assignment.
+    ///     A dedicated lock object: locking the array itself is unsound because the array
+    ///     reference is replaced on resize, after which concurrent creators lock different
+    ///     objects and race — duplicate world ids and lost slot writes (NREs in
+    ///     <see cref="EntityExtensions"/>, AccessViolation in <see cref="Chunk"/>).
+    /// </summary>
+    private static readonly object WorldsLock = new();
 
     /// <summary>
     ///     Stores recycled <see cref="World"/> IDs.
@@ -106,23 +121,31 @@ public static World Create(int chunkSizeInBytes = 16_384, int minimumAmountOfEnt
 #if PURE_ECS
         return new World(-1, chunkSizeInBytes, minimumAmountOfEntitiesPerChunk, archetypeCapacity, entityCapacity);
 #else
-        lock (Worlds)
+        lock (WorldsLock)
         {
             var recycle = RecycledWorldIds.TryDequeue(out var id);
             var recycledId = recycle ? id : WorldSize;
 
             var world = new World(recycledId, chunkSizeInBytes, minimumAmountOfEntitiesPerChunk, archetypeCapacity, entityCapacity);
 
-            // If you need to ensure a higher capacity, you can manually check and increase it
-            if (recycledId >= Worlds.Length)
+            var worlds = _worlds;
+            if (recycledId >= worlds.Length)
+            {
+                // Fill the slot in the copy before publishing the new array so readers
+                // (EntityExtensions and generated accessors index Worlds without a lock,
+                // and Entity handles carry an address dependency on the array reference)
+                // never observe a published array whose contents are not yet visible on
+                // weakly-ordered CPUs (ARM64).
+                var resized = new World[worlds.Length * 2];
+                Array.Copy(worlds, resized, worlds.Length);
+                resized[recycledId] = world;
+                Volatile.Write(ref _worlds, resized);
+            }
+            else
             {
-                var newCapacity = Worlds.Length * 2;
-                var worlds = Worlds;
-                Array.Resize(ref worlds, newCapacity);
-                Worlds = worlds;
+                Volatile.Write(ref worlds[recycledId], world);
             }
 
-            Worlds[recycledId] = world;
             Interlocked.Increment(ref worldSizeUnsafe);
             return world;
         }
@@ -533,9 +556,9 @@ protected virtual void Dispose(bool disposing)
         _isDisposed = true;
         var world = this;
 #if !PURE_ECS
-        lock (Worlds)
+        lock (WorldsLock)
         {
-            Worlds[world.Id] = null!;
+            Volatile.Write(ref _worlds[world.Id], null!);
             RecycledWorldIds.Enqueue(world.Id);
             Interlocked.Decrement(ref worldSizeUnsafe);
         }