migueldeicaza / SwiftGodot

New Godot bindings for Swift
https://migueldeicaza.github.io/SwiftGodotDocs/tutorials/swiftgodot-tutorials/
MIT License
985 stars 55 forks source link

Thread affinity for some tasks #471

Open migueldeicaza opened 2 months ago

migueldeicaza commented 2 months ago

Godot has some operations that are bound to certain threads, and you can searching ERR_THREAD_GUARD, ERR_MAIN_THREAD_GUARD, ERR_READ_THREAD_GUARD, and ERR_READ_THREAD_GUARD_V for methods in Node that we need to cover.

This for example shows up in connect of a signal, in particular when using the emitted pattern, as it requires that the method be involved not on the main thread, but on its thread group.

The following patch is a prototype that shows how this can work, and should be generalized to the generator, and we will need a list of APIs that we track with this thread information, so the generator produces the correct fences (main thread vs thread group):

commit 7814f48d7b546df9b2bbe602c6554aeeb0f825bb (HEAD -> signalOnThread, origin/signalOnThread)
Author: Miguel de Icaza <miguel@gnome.org>
Date:   Thu May 16 17:18:16 2024 -0400

    Potential fix, as we need to invoke connect from its thread group

diff --git a/Sources/SwiftGodot/Core/SignalSupport.swift b/Sources/SwiftGodot/Core/SignalSupport.swift
index 558c7e7..045a953 100644
--- a/Sources/SwiftGodot/Core/SignalSupport.swift
+++ b/Sources/SwiftGodot/Core/SignalSupport.swift
@@ -117,9 +117,18 @@ public class SimpleSignal {
         }

         let callable = Callable(object: signalProxy, method: SignalProxy.proxyName)
-        let r = target.connect(signal: signalName, callable: callable, flags: UInt32 (flags.rawValue))
-        if r != .ok {
-            print ("Warning, error connecting to signal \(signalName.description): \(r)")
+        if let tn = target as? Node {
+            _ = tn.callDeferredThreadGroup (
+                method: "connect",
+                Variant (signalName),
+                Variant (callable),
+                Variant (flags.rawValue)
+            )
+        } else {
+            let r = target.connect(signal: signalName, callable: callable, flags: UInt32 (flags.rawValue))
+            if r != .ok {
+                print ("Warning, error connecting to signal \(signalName.description): \(r)")
+            }
         }
         return signalProxy
     }

This manifest when using this idiom:

Task { @MainActor in
  GD.print("Awaiting animationFinished.")
  await self.animationPlayer?.animationFinished.emitted
  GD.print("Got animationFinished.")
}

Will trigger this error:

E 0:00:03:0953   connect: Caller thread can't call this function in this node (/root/Control/AnimatedRichTextLabel/@AnimationPlayer@3). Use call_deferred() or call_thread_group() instead.
  <C++ Error>    Condition "!is_accessible_from_caller_thread()" is true. Returning: (ERR_INVALID_PARAMETER)
  <C++ Source>   scene/main/node.cpp:3936 @ connect()

Because the animationFinished.emitted is a shortcut that called that from the wrong thread (the code lives in SignalSupport, but also will live in other signals)

We probably should cache the "connect" StringName above.