Try fix invalid PVS index bug (#5422)

* Try fix invalid PVS index bug

* bounds check

* More Asserts

* fix assert?

* remove deletion

* a

* A!
This commit is contained in:
Leon Friedrich
2024-09-16 16:12:20 +12:00
committed by GitHub
parent f5c1d870f9
commit f81e30a031
10 changed files with 35 additions and 16 deletions

View File

@@ -43,7 +43,13 @@ END TEMPLATE-->
### Bugfixes
<<<<<<< HEAD
* Fixed equality checks for `MarkupNode` not properly handling attributes.
* Fixed `MarkupNode` not having a `GetHashCode()` implementation.
* Fixed a PVS error that could occur when trying to delete the first entity that gets created in a round.
=======
* Fixed the "to" and "take" toolshed commands not working as intended.
>>>>>>> f5c1d870f904ca1a5d67ae6db20c17e181a26df9
### Other

View File

@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
@@ -190,6 +191,15 @@ internal struct PvsMetadata
private byte Pad0;
public uint Marker;
#endif
[Conditional("DEBUG")]
public void Validate(MetaDataComponent comp)
{
DebugTools.AssertEqual(NetEntity, comp.NetEntity);
DebugTools.AssertEqual(VisMask, comp.VisibilityMask);
DebugTools.AssertEqual(LifeStage, comp.EntityLifeStage);
DebugTools.Assert(LastModifiedTick == comp.EntityLastModifiedTick || LastModifiedTick.Value == 0);
}
}
[StructLayout(LayoutKind.Sequential, Size = 16)]

View File

@@ -245,8 +245,6 @@ internal sealed partial class PvsSystem
private void OnEntityAdded(Entity<MetaDataComponent> entity)
{
DebugTools.Assert(entity.Comp.PvsData.Index == default);
AssignEntityPointer(entity.Comp);
}
@@ -255,6 +253,7 @@ internal sealed partial class PvsSystem
/// </summary>
private void AssignEntityPointer(MetaDataComponent meta)
{
DebugTools.Assert(meta.PvsData == PvsIndex.Invalid);
if (_dataFreeListHead == PvsIndex.Invalid)
{
ExpandEntityCapacity();
@@ -267,8 +266,6 @@ internal sealed partial class PvsSystem
ref var freeLink = ref Unsafe.As<PvsMetadata, PvsMetadataFreeLink>(ref metadata);
_dataFreeListHead = freeLink.NextFree;
// TODO: re-introduce this assert.
// DebugTools.AssertEqual(((PvsMetadata*) ptr)->NetEntity, NetEntity.Invalid);
DebugTools.AssertNotEqual(meta.NetEntity, NetEntity.Invalid);
meta.PvsData = index;
@@ -287,9 +284,9 @@ internal sealed partial class PvsSystem
private void OnEntityDeleted(Entity<MetaDataComponent> entity)
{
var ptr = entity.Comp.PvsData;
entity.Comp.PvsData = default;
entity.Comp.PvsData = PvsIndex.Invalid;
if (ptr == default)
if (ptr == PvsIndex.Invalid)
return;
_incomingReturns.Add(ptr);

View File

@@ -49,7 +49,7 @@ namespace Robust.Server.GameStates
private void OnEntityDirty(Entity<MetaDataComponent> uid)
{
if (uid.Comp.PvsData != default)
if (uid.Comp.PvsData != PvsIndex.Invalid)
{
ref var meta = ref _metadataMemory.GetRef(uid.Comp.PvsData.Index);
meta.LastModifiedTick = uid.Comp.EntityLastModifiedTick;

View File

@@ -107,7 +107,7 @@ internal sealed partial class PvsSystem
internal void SyncMetadata(MetaDataComponent meta)
{
if (meta.PvsData == default)
if (meta.PvsData == PvsIndex.Invalid)
return;
ref var ptr = ref _metadataMemory.GetRef(meta.PvsData.Index);

View File

@@ -3,6 +3,7 @@ using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Robust.Shared.GameObjects;
using Robust.Shared.Timing;
using Robust.Shared.Utility;
namespace Robust.Server.GameStates;
@@ -25,6 +26,7 @@ internal sealed partial class PvsSystem
foreach (ref var ent in CollectionsMarshal.AsSpan(_cachedGlobalOverride))
{
ref var meta = ref _metadataMemory.GetRef(ent.Ptr.Index);
meta.Validate(ent.Meta);
if ((mask & meta.VisMask) == meta.VisMask)
AddEntity(session, ref ent, ref meta, fromTick);
}
@@ -51,6 +53,7 @@ internal sealed partial class PvsSystem
foreach (ref var ent in CollectionsMarshal.AsSpan(_cachedForceOverride))
{
ref var meta = ref _metadataMemory.GetRef(ent.Ptr.Index);
meta.Validate(ent.Meta);
if ((mask & meta.VisMask) == meta.VisMask)
AddEntity(session, ref ent, ref meta, fromTick);
}

View File

@@ -57,6 +57,7 @@ internal sealed partial class PvsSystem
foreach (ref var ent in span)
{
ref var meta = ref _metadataMemory.GetRef(ent.Ptr.Index);
meta.Validate(ent.Meta);
if ((mask & meta.VisMask) == meta.VisMask)
AddEntity(session, ref ent, ref meta, fromTick);
}
@@ -78,8 +79,7 @@ internal sealed partial class PvsSystem
if (meta.LifeStage >= EntityLifeStage.Terminating)
{
Log.Error($"Attempted to send deleted entity: {ToPrettyString(ent.Uid)}, lifestage is {meta.LifeStage}.\n{Environment.StackTrace}");
EntityManager.QueueDeleteEntity(ent.Uid);
Log.Error($"Attempted to send deleted entity: {ToPrettyString(ent.Uid)}, Meta lifestage: {ent.Meta.EntityLifeStage}, PVS lifestage: {meta.LifeStage}.\n{Environment.StackTrace}");
return;
}

View File

@@ -204,7 +204,7 @@ namespace Robust.Shared.GameObjects
/// <summary>
/// Offset into internal PVS data.
/// </summary>
internal PvsIndex PvsData;
internal PvsIndex PvsData = PvsIndex.Invalid;
}
[Flags]
@@ -254,5 +254,8 @@ namespace Robust.Shared.GameObjects
/// An invalid index. This is also used as a marker value in the free list.
/// </summary>
public static readonly PvsIndex Invalid = new PvsIndex(-1);
// TODO PVS
// Consider making 0 an invalid value.
// it prevents default structs from accidentally being used.
}
}

View File

@@ -15,5 +15,5 @@ public sealed class ToCommand : ToolshedCommand
[CommandArgument] ValueRef<T> end
)
where T : INumber<T>
=> Enumerable.Range(int.CreateTruncating(start), int.CreateTruncating(end.Evaluate(ctx)! - start)).Select(T.CreateTruncating);
=> Enumerable.Range(int.CreateTruncating(start), 1 + int.CreateTruncating(end.Evaluate(ctx)! - start)).Select(T.CreateTruncating);
}

View File

@@ -254,8 +254,8 @@ internal sealed unsafe class ResizableMemoryRegion<T> : IDisposable where T : un
public ref T GetRef(int index)
{
// If the memory region is disposed, CurrentSize is 0 and this check always fails.
if (index >= CurrentSize)
ThrowIndexOutOfRangeException();
if (index >= CurrentSize || index < 0)
ThrowIndexOutOfRangeException(CurrentSize, index);
return ref *(BaseAddress + index);
}
@@ -302,9 +302,9 @@ internal sealed unsafe class ResizableMemoryRegion<T> : IDisposable where T : un
}
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowIndexOutOfRangeException()
private static void ThrowIndexOutOfRangeException(int size, int index)
{
throw new IndexOutOfRangeException();
throw new IndexOutOfRangeException($"Index was outside the bounds of the memory region. Size: {size}, Index: {index}");
}
private void ReleaseUnmanagedResources()