Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.1.dev.3] Multiplayer game crash: multiplayer can only be manipulated from the main thread #77707

Closed
aldocd4 opened this issue May 31, 2023 · 4 comments

Comments

@aldocd4
Copy link

aldocd4 commented May 31, 2023

Godot version

v4.1.dev3.mono.official [a67d37f]

System information

Windows 11 - v4.1.dev3 - Vulkan

Issue description

Hello,

I'm playing with the latest 4.1 dev snapshot (3) and C#, and it seems that my multiplayer game crashes with the following stacktrace:

ERROR: Multiplayer can only be manipulated from the main thread.
   at: (scene/main/scene_tree.cpp:1477)
Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other me.
   at Godot.NativeInterop.NativeFuncs.godotsharp_method_bind_call(IntPtr, IntPtr, Godot.NativeInterop.godot_variant**, Int32, Godot.)
   at Godot.NativeCalls.godot_icall_3_676(IntPtr, IntPtr, Int64, Godot.NativeInterop.godot_string_name, Godot.Variant[])
   at Godot.Node.RpcId(Int64, Godot.StringName, Godot.Variant[])
   at FightHandler.ServerUpdateFightStep(Evael.Scripts.Game.Fight.Fight)
   at Evael.Scripts.Game.Fight.Fight.OnTimerTick(System.Object, System.Timers.ElapsedEventArgs)
   at System.Timers.Timer.MyTimerCallback(System.Object)
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Obje)
   at System.Threading.TimerQueueTimer.Fire(Boolean)
   at System.Threading.TimerQueue.FireNextTimers()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()

It's working correctly on 4.1 dev 1 and 4.1 dev 2 snapshots.

Steps to reproduce

A timer that trigger a simple function making an rpc call:

var timer = new Timer(14000);
timer.Elapsed += (object sender, ElapsedEventArgs e) =>
{
    RpcId(peerId, "UpdateFightStep", 1);
};
timer.AutoReset = true;
timer.Start();

Minimal reproduction project

I will try to reproduce it in a small project.

@aldocd4 aldocd4 changed the title Multiplayer game crash: multiplayer can only be manipulated from the main thread [4.1.dev.3] Multiplayer game crash: multiplayer can only be manipulated from the main thread May 31, 2023
@Chaosus Chaosus added this to the 4.1 milestone Jun 1, 2023
@aldocd4
Copy link
Author

aldocd4 commented Jun 6, 2023

Important note: the timer used here is the C# timer (System.Timers) and not the godot timer node.

I believe this bug occurs because the native C# timer is not single-threaded. It worked on latest v4.0 stable version and the 2 dev snapshots of 4.1.

Maybe this change is intended and is not a bug.

@AThousandShips
Copy link
Member

AThousandShips commented Jun 6, 2023

You need to use other methods as you are not allowed to manipulate it from the main thread, use something like deferred calls, this is because processing got reworked on 4.1.dev I believe

Don't think this is a bug

Edit: My bad, didn't realize this was get_multiplayer, I'm unsure why it has a thread check like that, that might be a bug yes

@raulsntos
Copy link
Member

The get_multiplayer method checks if we are in the main thread:

ERR_FAIL_COND_V_MSG(!Thread::is_main_thread(), Ref<MultiplayerAPI>(), "Multiplayer can only be manipulated from the main thread.");

This seems intentional, the PR says:

Multiplayer API is not thread safe.

The multiplayer code (rpc as example) is not thread safe, the Node API does not care about this, so we need to discuss if we want to implement safety at Node level or Multiplayer level.

Using a Godot timer should avoid the issue since it will tick in the main thread. But if you want to keep using C# timers, you should be able to use CallDeferred. You can use GodotObject.CallDeferred or a Callable, here's an example using a Callable:

var timer = new Timer(14000);
timer.Elapsed += (object sender, ElapsedEventArgs e) =>
{
    Callable.From(() => RpcId(peerId, "UpdateFightStep", 1)).CallDeferred();
};
timer.AutoReset = true;
timer.Start();

@YuriSizov
Copy link
Contributor

Closing per comment above (and OP's acknowledgement of it).

@YuriSizov YuriSizov closed this as not planned Won't fix, can't repro, duplicate, stale Jun 23, 2023
@YuriSizov YuriSizov removed this from the 4.1 milestone Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants