Skip to content

fix(vendor/msgpack-cs): patch to allow clientside serialization of types#5

Merged
prikolium-cfx merged 1 commit intocitizenfx:masterfrom
manups4e:master
Jun 6, 2025
Merged

fix(vendor/msgpack-cs): patch to allow clientside serialization of types#5
prikolium-cfx merged 1 commit intocitizenfx:masterfrom
manups4e:master

Conversation

@manups4e
Copy link
Contributor

@manups4e manups4e commented Jun 1, 2025

GOAL OF THIS PR

This patch serves as a temporary workaround, using #if !IS_FXSERVER checks, to address client-side serialization issues with formatters and their IL generating code.

Deserialization appears to work correctly and is untouched.

However, serialization consistently crashes the client, with a crash pointing to mono-2.0-sgen.dll+247C2.

While awaiting a proper, long-term fix, this patch introduces a straightforward fallback mechanism that bypasses the use of formatters during serialization on the client.

🧞‍♂️ Oh great @prikolium-cfx, I summon thee!
Grant us a wish: look into this arcane madness in mono library, and perhaps bestow upon us a fix worthy of the client realm (Or at least i can provide the dmp files to help you to help me checking into the problem).

  • If possible i'd prefer to fix the problem at its root instead of merging a patch to wait for a fix 🤔

What happens?

Client serialization of custom types crashes without stacktrace leaving no information at all to how to solve this mistery
image

trying to open the dmp leaves also no clues on the root cause of the problem
image

I'll keep investigating on the matter while this patch fixes temporarily the problem, allowing us to keep going forward in the testing process of experimental builds

Edit: it appears that everything comes down to

	// declaration
	internal delegate void MsgPackObjectSerializer(MsgPackSerializer serializer, object value);
	
	// delegate construction from formatters
        public Serializer(MethodInfo serializer, MethodInfo objectSerializer)
        {
	        m_method = serializer;
	        m_objectSerializer = (MsgPackObjectSerializer)objectSerializer.CreateDelegate(typeof(MsgPackObjectSerializer));
        }

        // code in MsgPackRegistry.Serialize()
        {...}
        else if (TryGetSerializer(type, out var methodInfo))
        {
	        methodInfo.m_objectSerializer(serializer, obj);
        }
        else
        {
	        var newSerializer = CreateSerializer(type);
	        if (newSerializer != null && newSerializer.Item1.m_objectSerializer != null)
		        newSerializer.Item1.m_objectSerializer(serializer, obj);
	        else
		        throw new SerializationException($"Type {type.Name} is not serializable");
        }

Somehow the m_objectSerializer delegate when called crashes the client, weirdly enough, deserialization which works the same way works perfectly.. what is happening here?

Fixes

Client side serialization of any formatter handled type

  • Dictionaries<,>
  • Lists<>, Arrays<>
  • Custom types

Copy link

@FabianTerhorst FabianTerhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to use spaces instead of tabs. Because you changed the code formatting.

feat(vendor/msgpack-cs): remove symbols from unit test project

fix(vendor/msgpack-cs): don't serialize player with client method in test

fix(vendor/msgpack-cs): fix unit tests

fix(vendor/msgpack-cs): patch to allow clientside serialization of types
}
}

public unsafe void Serialize(IEnumerable enumerable)
Copy link
Contributor Author

@manups4e manups4e Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the change of indentation from mixed to spaced... (thanks visual studio we love you...)
this is one of the changes:

  • Added for clientside IEnumerable serialization, handles Arrays, Lists, Dictionaries (ArrayFormatter and DictionaryFormatter)


#region Extra types

public unsafe void SerializeType(object obj)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the other change:
SerializeType patches serialization made in TypeFormatter for clientside

@FabianTerhorst
Copy link

Ok, the formatting seems to be mixed up even before this pull request. Better go with only tabs for now with every file.

@manups4e
Copy link
Contributor Author

manups4e commented Jun 2, 2025

yeah i always had mixed on one side.. and made it spaces now on the other side.. so i'll keep that memorized..
i commented the changed bits to highlight them

  • also, as i mentioned in the PR description.. i'd prefer to not merge this if we can find a solution to why formatters are sandboxed only in serialization while deserialization works perfectly fine.. that would be the best solution

@FabianTerhorst
Copy link

yeah i always had mixed on one side.. and made it spaces now on the other side.. so i'll keep that memorized.. i commented the changed bits to highlight them

* also, as i mentioned in the PR description.. i'd prefer to **not** merge this if we can find a solution to why formatters are sandboxed only in `serialization` while `deserialization` works perfectly fine.. that would be the best solution

If you can't resolve the serialization issue by yourself on the client then better merge it like this.

@manups4e
Copy link
Contributor Author

manups4e commented Jun 2, 2025

Alright, sounds good.
We can go ahead and merge this for now, so we can push forward the experimentation of the whole fixes.
I’ll keep investigating the serialization limitation on the client side and follow up with an new PR if I come up with a better solution.

@prikolium-cfx prikolium-cfx merged commit a296a76 into citizenfx:master Jun 6, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants