Thread does not start with a method with no arguments

[schweigert]

ElementaryOS 0.4.1 (Ubuntu 17) - Godot version: 2.1.3

Thread does not start with a method with no arguments
This node works:

extends Node
var thread = null

func _ready():
	thread = Thread.new()
	var err = thread.start(self, "work")

func work(userdata):
    print("oi")
    while(true):
        print("oi")

But this does not:

extends Node

var thread = null



func _ready():
	thread = Thread.new()
	var err = thread.start(self, "work")

func work():
    print("oi")
    while(true):
        print("oi")

I found this error during my question (https://godotengine.org/qa/16730/thread-not-running), but only by adding the argument did the method start to run.

[RandomShaper]

Well, I think it's the expected behavior.

You should be getting an error like this in the console so you get a clue on what's going on:

ERROR: Could not call function 'work'' starting thread ID: 33644 Reason: Too Many Arguments

[timoschwarzer]

I think Threads should still start even if there are no parameters defined in the Thread method signature.

1. Not starting the thread at all leads to confusion if you don't see/understand the error message
2. Often you just don't need an argument passed to your Thread

My proposal:

• If no arguments are given and the signature doesn't have any parameters defined → No error and start the thread
• If arguments are given in the .start method but no parameters are defined → Show an error, or even better crash the game with an error
• If no arguments are given but a parameter is defined → Run the thread with the parameter being null or []

[vnen]

I think it should match the default function call behavior: if the number of arguments in the call doesn't match the function arity, the game should break and the debugger open with an error.

IMO setting the parameters to null at the engine's may lead to confusion too, especially because Godot still can't debug inside threads.

[timoschwarzer]

@vnen Yes, but even if you call start() without passing arguments to the thread, the thread's method is still being called with one parameter, because the arguments are passed as an array. This is too confusing IMO.

Is it possible to pass the arguments as real parameters (not as an array) to the thread method?

[RandomShaper]

@timoschwarzer, I'm confused. Isn't the problem that if you pass no arguments then the thread gets no arguments as well and that causes an error?

Or do you mean passing an empty array?

[timoschwarzer]

No, if you pass no arguments the parameter in the start method defaults to an empty array.

The OP's problem is, that the thread doesn't start even if there are no arguments given. This is intended.

Now we can discuss if the thread should start if the thread method has no parameters defined (func work():) and no arguments are given (thread.start(self, "work")) for convenience reasons and to prevent confusion for beginners.

[akien-mga]

Is it possible to pass the arguments as real parameters (not as an array) to the thread method?

Why? The start() method takes an array of arguments, because the target method can have 0 or more arguments. So for work(userdata), you need to call thread.start(self, "work", [some_userdata]), just like you would call work(some_userdata) outside of a thread.

If you method is work(userdata, answer), then you'd call thread.start(self, "work", [some_userdata, 42]).

Now we can discuss if the thread should start if the thread method has no parameters defined (func work():) and no arguments are given (thread.start(self, "work")) for convenience reasons and to prevent confusion for beginners.

Well, AFAIU, it already works, no? If there's no parameters required, you pass no arguments aka an empty array.

[timoschwarzer]

No, AFAIK the parameters are passed as an array to the method.
So when you pass no extra arguments to the start() method it will pass an empty array to the thread method.

[akien-mga]

So when you pass no extra arguments to the start() method it will pass an empty array to the thread method.

No, it's an array of parameters, not the parameter itself. Empty array means no parameters. If it behaves differently, it's a bug.

[RandomShaper]

This:

func _ready():
	var thread = Thread.new()
	thread.start(self, "work", [11, 22, 33])
	thread.wait_to_finish()

func work(args):
	print(args)

prints:

[11, 22, 33]

[akien-mga]

Ok, then that's a bug IMO. It should be:

func _ready():
	var thread = Thread.new()
	thread.start(self, "work", [11, 22, 33])
	thread.wait_to_finish()

func work(arg1, arg2, arg3):
	print(arg1, arg2, arg3)

prints:

11 22 33

i.e. just like connect() or call_deferred() work.

[RandomShaper]

I think this is modeled after the usual threading APIs limitation of being able to pass only one user data varaible.

Of course, Godot could abstract that out, but it makes sense to me. Anyway if thas was going to be changed, it should be done for 3.0.

[timoschwarzer]

Then the doc is wrong, too:

Start a new Thread, it will run “method” on object “instance” using “userdata” as an argument and running with “priority”, one of PRIORITY_* enum.

[akien-mga]

I think this is modeled after the usual threading APIs limitation of being able to pass only one user data varaible.

Ah well if that's how threads are usually designed, we should likely keep it like that.

[timoschwarzer]

If we keep it like that we should consider allowing 0 parameters for convenience reasons and to prevent confusion.

[RandomShaper]

POSIX threads:

int pthread_create(
pthread_t *thread,
const pthread_attr_t *attr,
void *(*start_routine) (void *),
void *arg); <--- One arg

Windows threads:

HANDLE WINAPI CreateThread(
In_opt LPSECURITY_ATTRIBUTES lpThreadAttributes,
In SIZE_T dwStackSize,
In LPTHREAD_START_ROUTINE lpStartAddress,
In_opt LPVOID lpParameter, <--- One arg
In DWORD dwCreationFlags,
Out_opt LPDWORD lpThreadId
);

C++11 threads seem to allow for multiple arguments, but I've not checked them personally.

[RandomShaper]

Anyway. it doesn't have to be an array. You can pass a plain variable if you don't need multiple values.

[RandomShaper]

But about the error reporting. What about?:

• Thread.stat() with no third argument or explicit null:
  • worker_fn() with one or more arguments => Error message and debugger breaks.
  • worker_fn() with no arguments => OK
• Thread.start() with non-null third argument:
  • worker_fn() with one argument => OK
  • worker_fn() with no arguments or more than one => Error message and debugger breaks.

[akien-mga]

Sounds good, though:

• Thread.stat() with no third argument or explicit null:
  • worker_fn() with one or more arguments => Error message and debugger breaks.

I guess null could be valid input for worker_fn() with one argument, no? (e.g. you could be passing a collider Object that might be null at times). It's the method's role to validate its input IMO.

[RandomShaper]

That's right.

There would be an ambiguity about explicit vs. default null, but not much can be done about it.

[akien-mga]

So I guess in the end we should just enforce one argument in all cases and just make the debugger helpful enough?

[RandomShaper]

I think so, because the error shouldn't depend on passing null or non-null. I mean, if the worker fn had no arguments and the calling code were wrongly passing some value in, sometimes null but non-null on edge cases, you may overlook the problem.

In other words, you would be loosing a chance of dynamic analysis of the code. At least, with the parameter enforcement, you can know you are doing something wrong as soon as start() is called.

[RandomShaper]

I was about to implement this, but something is not clear to me.

In order to let the debugger break, I think I should call GDScriptLanguage::debug_break() since there is the logic for breaking only if called from the main thread and also it will pass the error location, etc. to the debugger. But then I'd be adding GDScript-specific stuff to the language-agnostic Thread bindings.

Should this behavior of letting the debugger kick in be only available when using Thread from GDScript, provided other languages will have their own implementation of threads? But anyway that would involve adding a check for that in the Thread's code, which doesn't seem so elegant either.

What do you think?

[teedoubleuu]

Its extremely confusing to have the Thread.start() fail if the method called in the thread has no arguments. If I don't pass any userdata to the Thread.start() method then it should not try passing an empty array to the method I'm calling in the thread.

[akien-mga]

Still valid in the 3.x branch as per #18235.

[Nolkaloid]

I think that it's a good idea to allow multiple and no parameters, RN you must run in on a "compatibility" function that executes the final wanted function with your correct (or no) arguments, so the best would be to pass arguments as an array:

func _ready():
      var thread = Thread.new()
      thread.create(self, "method", ["Hello World ", 2, 4.20])

func method(text, x, y):
      print(String, x*y)

Output:

Hello World 8.4

[bojidar-bg]

An option would be to make Thread.start a vararg method.

[schweigert]

I like the idea of having a reserved word to execute a thread automatically in the language itself.

func _ready():
      var thread = method("blabla", 2, 3)
      # Thread.new().create(self, "method", ["blabla", 2, 3])

async method(text, x, y):
      print(text)
      print(x * x + y)

[timoschwarzer]

@schweigert
I think, if we introduce a new keyword for threads, it should be located at the function invocation rather than at the function itself.

func _ready():
    var thread = async my_method(1, 2, 3)
    var no_thread = my_method(1, 2, 3)

func my_method(a, b, c):
    # do work
    pass

This would still allow us to call the method synchronously. I don't know if it's worth a new syntax sugar keyword though.

[schweigert]

@timoschwarzer I think so. New languages like Kotlin and Golang are using parallel programming in this way.

[vnen]

@schweigert it's better if you open a new issue with your suggestion.

[Norrox]

Was making a thread in my demo project today and i made a thread with no argument and ofc it did not work, and then after awhile i remembered this error :)

[aaronfranke]

Did someone fix this bug? It seems the workaround in the Voxel demo isn't necessary in 3.4. godotengine/godot-demo-projects#639

[schweigert]

It do not work today @aaronfranke

[akien-mga]

@schweigert What version did you try? It is very likely fixed in the latest 3.x branch with #38078 and #51093.

[akien-mga]

I can confirm that it's fixed in 3.4 beta 2 (so it was fixed by #38078).