FreeBSD: add pthread_getthreadid_np()#2725
Conversation
SteveLauC
left a comment
There was a problem hiding this comment.
Thanks for the PR. A changelog entry is needed, BTW, see https://github.com/nix-rust/nix/blob/master/CONTRIBUTING.md#pull-requests
src/unistd.rs
Outdated
There was a problem hiding this comment.
| #[cfg(freebsd)] | |
| #[cfg(target_os = "freebsd")] |
We only use the alias when it contains more than 2 OSes
| /// Get the caller's thread ID (see | ||
| /// [pthread_getthreadid_np(3)](https://man.freebsd.org/cgi/man.cgi?query=pthread_getthreadid_np&sektion=3&manpath=FreeBSD+15.0-RELEASE+and+Ports). | ||
| #[cfg(freebsd)] | ||
| #[inline] |
There was a problem hiding this comment.
| #[inline] | |
| #[inline] | |
| #[doc(alias = "pthread_getthreadid_np")] |
Maybe we should add an alias 👀
There was a problem hiding this comment.
If we use its own name, this alias is not needed.
See the below comment
src/unistd.rs
Outdated
There was a problem hiding this comment.
Open question, should we call it gettid(), i.e., follow the Linux convention here, or we should use its own name?
I am slightly inclined to use its original name as Nix, in most cases, only exposes system stuff in Rusty way, we do not wrap them
There was a problem hiding this comment.
I added it as gettid() because this is what some consumers need, that depend on nix with features = ["process"]. Using pthread_gettreadid_np() there would not allow e.g. perfetto-recorder to compile.
There was a problem hiding this comment.
Using pthread_gettreadid_np() there would not allow e.g. perfetto-recorder to compile.
Well, this is unfortunate. But Nix does not provide cross-platform wrappers. Typically, they will be provided by crates that depend on Nix. So I still think we should use the original name: pthread_getthreadid_np :>
There was a problem hiding this comment.
I agree. In the past Nix tried to provide a measure of portability. But it didn't work well. It's better when concerns are separated: Nix provides Rusty interfaces to OS APIs, and other crates atop Nix provide cross-platform abstractions.
There was a problem hiding this comment.
Without this specific name, the patch is not very useful. The delivery path become this, then wait for next nix release, then push the change to that crate, then wait for its release.
There was a problem hiding this comment.
Well, I can make a release right now, if you need one.
There was a problem hiding this comment.
Not that I need one, but it would be nice to have, some day.
c865442 to
5ff9695
Compare
All done, thanks for the review. |
What does this PR do
The process::gettid() is missing on FreeBSD.
The PR adds its implementation.
The implementation returns the real tid of the caller's thread, it does
not try to mimic the Linux behavior where initial (main) thread id is
same as the processs id. This might be patched if needed, but pids and
tids are in different namespace on FreeBSD, so better keep it as is
unless there is a pressure.
Checklist:
CONTRIBUTING.md