sbox-issues icon indicating copy to clipboard operation
sbox-issues copied to clipboard

Syncing a struct is causing a stack overflow

Open ShaunLen opened this issue 7 months ago • 5 comments

Describe the bug

After lots of debugging as a group, we've narrowed the issue down as best we can. We have a struct that has a [Sync(SyncFlags.FromHost)] attribute, which results in a stack overflow in the TryGetValue method of the ConcurrentDictionary class. Everything works perfectly fine without the sync attribute, and works with the sync attribute if this struct is instead a class.

We are using other structs in the same way without issue and are unable to identify any difference that could be causing this.

To Reproduce

As the issue is so specific, best way to reproduce would be if we give you access to the project repo. Happy to hop into a call with whoever investigates to provide context. I'll message conna to let him know this has been raised.

Expected behavior

Structs should be able to sync without issue.

Media/Files

No response

Additional context

No response

ShaunLen avatar May 17 '25 17:05 ShaunLen

Sync is only supported inside a component class

https://sbox.game/dev/doc/networking-multiplayer/sync-properties/

GavrilovNI avatar May 17 '25 17:05 GavrilovNI

Sync is only supported inside a component class

https://sbox.game/dev/doc/networking-multiplayer/sync-properties/

Yes, the instance of the struct that we are syncing is inside of a component.

ShaunLen avatar May 17 '25 17:05 ShaunLen

Sync is only supported inside a component class

https://sbox.game/dev/doc/networking-multiplayer/sync-properties/

Yes, the instance of the struct that we are syncing is inside of a component.

So you have a property of that struct inside a component?

Is that struct unmanaged? Only unmanaged structs are supported Also classes aren't supported at all. If it is working with class, you should not rely on it.

GavrilovNI avatar May 17 '25 18:05 GavrilovNI

Also if you have recursion in that struct(same struct inside of a struct), I imagine its also not supported, unless inner struct is nullable. I think this is what you have cause you get stack overflow.

GavrilovNI avatar May 17 '25 18:05 GavrilovNI

Can you share your struct code ? Maybe more easier to know what can cause the issue

bub-bl avatar May 18 '25 07:05 bub-bl

Might be fixed now on staging, if not could you send your struct like @bub-bl suggested.

Metapyziks avatar May 20 '25 12:05 Metapyziks

Thanks @Metapyziks

public record struct Character
{
	[JsonPropertyName("id")]
	public Guid Id { get; set; }

	[JsonPropertyName("steam_id")]
	public long SteamId { get; set; } // FK

	[JsonPropertyName("forename")]
	public string Forename { get; set; }

	[JsonPropertyName("surname")]
	public string Surname { get; set; }

	[JsonIgnore]
	public string FullName => $"{Forename} {Surname}";

	[JsonPropertyName("ssn")]
	public string Ssn { get; set; }

	[JsonPropertyName("phone_number")]
	public string PhoneNumber { get; set; }

	[JsonPropertyName("clothing")]
	public string Clothing { get; set; }

	public Character(Guid id, string forename, string surname, string ssn, string phoneNumber, string clothing)
	{
		Id = id;
		Forename = forename;
		Surname = surname;
		Ssn = ssn;
		PhoneNumber = phoneNumber;
		Clothing = clothing;
	}
	
	public bool IsValid() => SteamId != 0;
	public static Character NotFound => default;
}

This is the struct that was previously causing the editor to crash when synced. It's now working totally fine on staging. As @GavrilovNI mentioned above, managed structs are not supported for sync - having strings like we do above makes the struct a managed type.

Interestingly, however, the property below manages to sync without issue (that I can see):

[Sync( SyncFlags.FromHost )]
public Character Data { get; private set; }

Is this supposed to work, or is there something that could/will go wrong if I continue using it in this way? Since it seems to be working just fine, my assumption is that whatever trickery allows string properties to be directly synced also applies to string properties within structs.

ShaunLen avatar May 20 '25 13:05 ShaunLen

We'll aim to support all structs where all the individual fields are also supported networkable types. Going forward, if you find a struct that meets that criteria but doesn't work, it's a bug.

Your Character struct probably wasn't working before because our network serialization logic was pretty broken. It would have tried to serialize the static NotFound property, which was also a Character and would have recursed.

Metapyziks avatar May 20 '25 13:05 Metapyziks