UnmanagedNetworkSerializableSerializer Duplicate Method Causes Errors with Disposable Types Due to Pure Assignment

[harayuu9]

Description

The Duplicate method in UnmanagedNetworkSerializableSerializer causes errors when handling types that implement IDisposable due to pure assignment. This leads to issues such as multiple Dispose calls on NativeArray, resulting in runtime errors.

Reproduce Steps

1. Create a struct that implements IDisposable and INetworkSerializable.
2. Use this struct as a type for a NetworkVariable.
3. Perform operations that involve serialization and deserialization of this struct.
4. Observe the errors related to multiple Dispose calls.

Actual Outcome

When using a struct that implements IDisposable with NetworkVariable, the Duplicate method causes multiple Dispose calls on NativeArray, leading to runtime errors and instability.

Expected Outcome

The Duplicate method should handle types that implement IDisposable correctly, ensuring that Dispose is called only once, avoiding multiple Dispose calls and ensuring stable runtime behavior.

Environment

• OS: [e.g. macOS Monterey]
• Unity Version: [e.g. 2023.1.20]
• Netcode Version: [e.g. 1.9.1]

Additional Context

Example of the problematic class:

public struct NativeArray2D<T> : IDisposable, INetworkSerializable
    where T : unmanaged
{
    private int _width;
    private int _height;
    private NativeArray<T> _array;
    
    public NativeArray<T>.ReadOnly RawArray => _array.AsReadOnly();
    public readonly int Width => _width;
    public readonly int Height => _height;
    public int Length => _array.Length;
    
    public NativeArray2D(int width, int height, Allocator allocator)
    {
        _array = new NativeArray<T>(width * height, allocator);
        _width = width;
        _height = height;
    }
    
    public NativeArray2D(int width, int height, Allocator allocator, T defaultValue) : this(width, height, allocator)
    {
        for (var i = 0; i < _array.Length; i++)
        {
            _array[i] = defaultValue;
        }
    }
    
    public T this[int x, int y]
    {
        get => _array[x + y * Width];
        set => _array[x + y * Width] = value;
    }
    
    public void Dispose()
    {
        _array.Dispose();
        _array = default;
    }

    public void NetworkSerialize<T1>(BufferSerializer<T1> serializer) where T1 : IReaderWriter
    {
        serializer.SerializeValue(ref _width);
        serializer.SerializeValue(ref _height);

        if (serializer.IsWriter)
        {
            if (_array.IsCreated)
            {
                serializer.GetFastBufferWriter().WriteValueSafe(true);
                serializer.SerializeValue(ref _array, Allocator.Persistent);
            }
            else
            {
                serializer.GetFastBufferWriter().WriteValueSafe(false);
            }
        }
        else
        {
            serializer.GetFastBufferReader().ReadValueSafe(out bool isCreated);
            if (isCreated)
            {
                serializer.SerializeValue(ref _array, Allocator.Persistent);
            }
        }
    }
}

Solution

The current implementation of the Duplicate method in UnmanagedNetworkSerializableSerializer does not handle disposable types correctly because it uses pure assignment. To address this issue, consider using UserNetworkVariableSerialization's Duplicate method if available, or implement a custom serialization mechanism similar to the managed approach. For example:

if (UserNetworkVariableSerialization.Duplicate != null)
{
    return UserNetworkVariableSerialization.Duplicate(value);
}
else
{
    using var writer = new FastBufferWriter(256, Allocator.Temp, int.MaxValue);
    var refValue = value;
    Write(writer, ref refValue);

    using var reader = new FastBufferReader(writer, Allocator.None);
    Read(reader, ref duplicatedValue);
    return duplicatedValue;
}

Disposable structs, even if unmanaged, should be handled separately to ensure proper resource management. This prevents issues related to multiple Dispose calls and ensures stable operation of the networked application.

[harayuu9]

If the copy in the related case was properly duplicated, NetworkVariableSerialization.ValueEquals would also behave unexpectedly. (Since the Ptr of NativeArray becomes different, Equal will not pass from the moment it is duplicated) The essence remains unchanged, it is an issue caused by not considering disposable unmanaged types.

internal static unsafe bool ValueEquals<TValueType>(ref TValueType a, ref TValueType b) where TValueType : unmanaged
{
    // get unmanaged pointers
    var aptr = UnsafeUtility.AddressOf(ref a);
    var bptr = UnsafeUtility.AddressOf(ref b);

    // compare addresses
    return UnsafeUtility.MemCmp(aptr, bptr, sizeof(TValueType)) == 0;
}