From 0b2ee093788ba84c5d130c021f347844b5939889 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Tue, 21 Feb 2023 18:09:00 +0100 Subject: [PATCH] std.Thread.setName: use unused code I noticed a comment saying that the intent of a code's author was unclear. What happened is that the author forgot to put the check for whether the thread is the calling thread (`self.getHandle() == std.c.pthread_self()`) in the `if (use_pthreads)`. If the thread is the calling thread, we use `prctl` to set or get the thread's name and it does not take a thread id because it knows the id of the thread we're calling `getName` or `setName` from. I have found a source saying that using `pthread_setname_np` on either the calling thread or any other thread by thread id would work too (so we don't need to call `prctl`) but I was not sure if that is the case on all systems so we keep using `pthread_setname_np` if we have a specific thread that is not the thread we're calling from, and `prctl` otherwise. --- lib/std/Thread.zig | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/std/Thread.zig b/lib/std/Thread.zig index 8004f94d7f..a0f66b3fa8 100644 --- a/lib/std/Thread.zig +++ b/lib/std/Thread.zig @@ -62,18 +62,20 @@ pub fn setName(self: Thread, name: []const u8) SetNameError!void { switch (target.os.tag) { .linux => if (use_pthreads) { - const err = std.c.pthread_setname_np(self.getHandle(), name_with_terminator.ptr); - switch (err) { - .SUCCESS => return, - .RANGE => unreachable, - else => |e| return os.unexpectedErrno(e), - } - } else if (use_pthreads and self.getHandle() == std.c.pthread_self()) { - // TODO: this is dead code. what did the author of this code intend to happen here? - const err = try os.prctl(.SET_NAME, .{@ptrToInt(name_with_terminator.ptr)}); - switch (@intToEnum(os.E, err)) { - .SUCCESS => return, - else => |e| return os.unexpectedErrno(e), + if (self.getHandle() == std.c.pthread_self()) { + // Set the name of the calling thread (no thread id required). + const err = try os.prctl(.SET_NAME, .{@ptrToInt(name_with_terminator.ptr)}); + switch (@intToEnum(os.E, err)) { + .SUCCESS => return, + else => |e| return os.unexpectedErrno(e), + } + } else { + const err = std.c.pthread_setname_np(self.getHandle(), name_with_terminator.ptr); + switch (err) { + .SUCCESS => return, + .RANGE => unreachable, + else => |e| return os.unexpectedErrno(e), + } } } else { var buf: [32]u8 = undefined; @@ -177,6 +179,7 @@ pub fn getName(self: Thread, buffer_ptr: *[max_name_len:0]u8) GetNameError!?[]co else => |e| return os.unexpectedErrno(e), } } else if (use_pthreads and self.getHandle() == std.c.pthread_self()) { + // Get the name of the calling thread (no thread id required). const err = try os.prctl(.GET_NAME, .{@ptrToInt(buffer.ptr)}); switch (@intToEnum(os.E, err)) { .SUCCESS => return std.mem.sliceTo(buffer, 0), @@ -325,10 +328,10 @@ pub const SpawnError = error{ Unexpected, }; -/// Spawns a new thread which executes `function` using `args` and returns a handle the spawned thread. +/// Spawns a new thread which executes `function` using `args` and returns a handle to the spawned thread. /// `config` can be used as hints to the platform for now to spawn and execute the `function`. /// The caller must eventually either call `join()` to wait for the thread to finish and free its resources -/// or call `detach()` to excuse the caller from calling `join()` and have the thread clean up its resources on completion`. +/// or call `detach()` to excuse the caller from calling `join()` and have the thread clean up its resources on completion. pub fn spawn(config: SpawnConfig, comptime function: anytype, args: anytype) SpawnError!Thread { if (builtin.single_threaded) { @compileError("Cannot spawn thread when building in single-threaded mode");