Let user scripts disable thread safety checks

[RandomShaper]

The new thread guards are too strict for old-school threading. There are legit usages for it that the engine would now block. This PR adds a mechanism to bypass the checks. They were, after all, designed to ensure thread safety during group processing. The fact that they also yell on arbitrary threads is a side effect. It's a mostly beneficial one, but too conservative in some cases.

See #77974 (comment).

Bugsquad edit: Fixes #77764

[Fran6nd]

My dream PR! Since two weeks of thread safety implementation, all my project is now completely broken and I am having a lot of troubles to bypass them. Happy to see this coming, hope it will be merged.

[Calinou]

Does this resolve godot-extended-libraries/godot-debug-menu#4?

[RandomShaper]

Does this resolve godot-extended-libraries/godot-debug-menu#4?

I think so.

This should be a relevant thing to mention in the release notes.

For the records, I was wondering if we could have just disabled thread safety checks by default in user tasks/threads, but I think that, given the engine is evolving towards more exhaustive checks and requiring users to be at least aware of entering danger zone when they wouldn't know formerly, a explicit request to disable the checks seems better to me. An alternative would be a project setting, though, but I guess this level of explicitness is better to have segments of script somehow flagged as potentially unsafe.

[KoBeWi]

The node threading changes broke all my code that used threads, even if the code was working perfectly and was relatively safe. I managed to fix the behavior, but wasn't able to get rid of the error spam. This PR allowed me to fix all problems. I think it would be nice to have it in 4.1, because many users will likely run into the same issues.

[akien-mga]

I don't know much about thread usage both internally and in user scripts, so TIWAGOS, but I'm trying to understand the impact of this change on user code.

This is a static method, so does this mean users will call this once with Thread.set_thread_safety_checks_enabled(false) and opt out of all safety checks for all their Thread usage? Does this affect only the threads they create themselves, or does it also disable thread safety checks for the SceneTree MT groups?

Are there situations where user instantiated Threads should still use those safety checks? If not, would it make sense to separate Thread into two classes, so user instantiated Threads don't change behavior compared to 4.0, but a new SceneTreeThread would be used internally with the safety checks for nodes?

[KoBeWi]

Well, I used this method like this:

func something():
	thread = Thread.new()
	thread.start(something_thread)

func something_thread():
	Thread.set_thread_safety_checks_enabled(false)

From what I understand, the new safety checks exist for the threading Node usage, i.e. you have Nodes in different thread groups and the checks ensure that Nodes interact without exploding. Disabling checks is useful when you have an isolated code that you know is thread safe.

The way I imagine it, which actually doesn't really sound right, is that you write code with thread checks to make sure it works correctly and then disable checks to get rid of error spam. I mean, my code stopped working after upgrading to 4.1; I managed to make it working again, which probably made it more safe, but I was not able to remove all the errors. And the errors, from my experience, don't really indicate your code is wrong, because you might be using threads in a context which is somewhat thread-safe (e.g. #77764).

[RandomShaper]

Yeah, the checks were added for the group processing, but can be useful as well for user-started threads and user tasks in the WorkerThreadPool. I mean, the safety model of node group processing is a guaranteed way of having thread safety and users can embrace it if they want to play nicely with the engine. With checks disabled, thread safety may still be possible, but the engine has no idea if what the user is doing is actually safe or not. Moreover, a user thread with checks disabled can interfere with threaded node processing, making the interaction unsafe.

Aside, @akien-mga, the control of the checks is, as @KoBeWi confirms, for the calling thread. But, now you mention it, it'd be possible from user scripts to mark the main thread as unsafe, which would make no sense. I have to add a check there. Otherwise, the only other engine-owned threads that the user would be able to tweak safety on are those of the WorkerThreadPool, but that's legit and the pool will reset the flag upon completion of the user task.

I'm not sure if this covers all comments.

[RandomShaper]

Done.

[KoBeWi]

I didn't test if it's possible, and it's probably not worth handling, but a user might also disable checks in node group thread, which isn't main, by calling set_thread_safety_checks_enabled() e.g. in _process() method of a Node outside main group group.

[RandomShaper]

I didn't test if it's possible, and it's probably not worth handling, but a user might also disable checks in node group thread, which isn't main, by calling set_thread_safety_checks_enabled() e.g. in _process() method of a Node outside main group group.

I've explained in the docs that guards for group processing will be in place regardless. However, in the future maybe non-node classes have more universal guards that don't diverge between group processing and other contexts. So, I indeed believe it's a good idea to explicitly error if user attempts that

[akien-mga]

Thanks!

[Bonkahe]

So I have a issue regarding this and specifically C#.
My issue is that with resources which are not going to change during gameplay, throw endless errors regarding being accessed by threads, I cannot find any way in C# to fix this, there does not appear to be a method for Thread.set_thread_safety_checks_enabled() or it's logical counterpart GodotThread.SetThreadSafetyChecksEnabled() nor does there appear to be any ways to decorate resources so as to flag them as being thread safe, is there any solution to this currently @RandomShaper ? I do apologize for directly @'ing you in this manner but I was unable to get any sort of satisfactory response on the discord, and you seem to be the only one who knows about this subject in any great detail.

[akien-mga]

@Bonkahe Which Godot version are you using exactly? This was merged after 4.1 beta 1 so it's not yet in any public release.

[Bonkahe]

Currently on the Godot 4.1-beta1-mono from tuxfamily mirror, (I install and manage my project using Godot Manager for what it's worth)
Should I try and build from source? I believe the TuxFamily build was done on June 7th.

[akien-mga]

You can build from source, or wait for 4.1 beta 2 which is scheduled for tomorrow.

[Bonkahe]

What is this patience you tell me of?
Lol I will wait until tomorrow, thank you so much for helping out!