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

Can't call emit_signal() within a Thread in 4.X. #81148

Open
KnightNine opened this issue Aug 30, 2023 · 5 comments
Open

Can't call emit_signal() within a Thread in 4.X. #81148

KnightNine opened this issue Aug 30, 2023 · 5 comments

Comments

@KnightNine
Copy link

Godot version

4.1

System information

Windows 10

Issue description

Simple as the title describes, not sure if this is intended now but it sure is annoying that it's no longer possible.

The Error that is Thrown:

E 0:00:02:0389   Node::emit_signalp: Caller thread can't call this function in this node (/root/Thread Hooking). 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:3563 @ Node::emit_signalp()

Steps to reproduce

the code which throws said error:

var thread = Thread.new()
signal thread_frame


func thread_process():
	emit_signal("thread_frame")
		

func _ready():
	var c = Callable(self,"thread_process")
	thread.start(c)

Minimal reproduction project

N/A

@KnightNine
Copy link
Author

I didn't realize #78000 was already merged.
It solves this issue.

I guess it's intended that signals are not thread safe.
It would be useful if the ThreadSafeAPIs page in the docs mentioned that signals and await calls (as per #78149 ) were not thread safe and pointed users towards how to use: Thread.set_thread_safety_checks_enabled(false)

@akien-mga
Copy link
Member

I don't think disabling thread safety checks is correct here. You should defer emitting the signal as suggested by the error.

@KnightNine
Copy link
Author

KnightNine commented Aug 30, 2023

@akien-mga
I suppose for most contexts I would do that if I were just trying to signal something.

Though for the very specific purpose that I'm using it for, awaiting/yielding on the thread directly is required.

Come to think of it, there may not be many contexts where this is absolutely needed.
IDK, mods can close this issue if they don't see the need to add more info to the ThreadSafeAPIs page.

@bitsawer
Copy link
Member

It's possible the new thread safety check is correct, but it's also possible it's a bit too strict. It's a relatively new feature, so we might have missed something. These kind of issues were the reason Thread.set_thread_safety_checks_enabled() was added in #78000. For example similar issues #77974 was a case of too strict thread guard we didn't notice, possibly #80752 too.

But we probably need someone from the core team to evaluate these thread guard issues. For the record, this is where the error check fails:

ERR_THREAD_GUARD_V(ERR_INVALID_PARAMETER);

@SanitoGonzalez
Copy link

SanitoGonzalez commented Jul 15, 2024

In GDExtension C++, I got the same error while trying to call emit_signal in the new std::thread other than godot main thread.

Change from

emit_signal("my_signal", 42);

to

 call_deferred("emit_signal", "my_signal", 42);

worked for me.

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