Thread pool spawn thread missing allocator parameter cause panic in wasi

[JiaJiaJiang]

https://github.com/ziglang/zig/blob/8573836892ba1b7cd34d377b46258930161256c3/lib/std/Thread/Pool.zig#L57

Here the spawn method dose not pass the allocator in, but wasi thread must need an allocator: https://github.com/ziglang/zig/blob/8573836892ba1b7cd34d377b46258930161256c3/lib/std/Thread.zig#L919-L922

so it panics:thread 0 panic: an allocator is required to spawn a WASI thread.

I changed this line to :

thread.* = try std.Thread.spawn(.{ .allocator = allocator }, worker, .{pool});

and it worked.

May this be fixed in std lib?

---

Edit: Maybe my attempt above is not the best solution. Although I don’t know why, I found that the performance of the thread pool generated in this way is much lower than manually created threads.

---

Edit2: There are also some other calls of std.Thread.spawn that not passing an allocator, some of them may need to be fixed too.

[Rexicon226]

noting #19383 exists

to add, assuming we do continue taking in an allocator to spawn, I think the struct should only have an allocator field if the target OS is wasi. that would prevent this issue from happening since it would give an "allocator field not given" error, and it would remove the potentially confusing field that isn't used anywhere else.

[JiaJiaJiang]

@Rexicon226 OK, I see. It seems that the main problem is how to free heap memory allocated by threads? So should I close this issue? Or just leave it open since there are already tags and milestone attached on it.

[Rexicon226]

The main problem is the current API doesn’t enforce an allocator argument and creates a runtime panic when that doesn’t need to be the case. The usage in the threadpool would have been caught if it were designed as I propose.

Don’t close the issue, I think this is a pretty easy bug to fix for any contributors looking to help out.