Add gettid

[dhylands]

I tested this under linux, and I noticed that this seems to also be built for OSX. It would be appreciated if someone could test this under OSX.

I'm not familiar enough with rust to know if there is a way of integrating this without creating a sub-crate.

[posborne]

Although OSX has gettid, it appears to be not the same thing and I question whether we should expose it for OSX at all: http://elliotth.blogspot.com/2012/04/gettid-on-mac-os.html.

gettid should be exposed from libc. We should also update unistd to use the libc getpid and getppid.

[posborne]

It should be noted that libc does expose pthread_self(): https://doc.rust-lang.org/libc/x86_64-unknown-linux-gnu/libc/fn.pthread_self.html

Use of pthread_self is probably preferable in many cases, especially where some degree of portability is desired (there are still gotchas). It might be reasonable for nix to be extended to include a wrapping of pthread that adds the typical nix niceties.

That being said, I can still imagine use cases where getting access to gettid() would be nice. I think it should be added for Linux in libc -- it should also be a direct syscall as glibc does not provide any wrapper according to the man pages (http://man7.org/linux/man-pages/man2/gettid.2.html).

[kamalmarhubi]

I think it should be added for Linux in libc -- it should also be a direct syscall as glibc does not provide any wrapper according to the man pages (http://man7.org/linux/man-pages/man2/gettid.2.html).

I am not sure it can be added to libc as a function in that case. The syscall number could and should be added though. We have a similar situation with pivot_root(2), where nix wraps a call to syscall(2).

[kamalmarhubi]

We should also update unistd to use the libc getpid and getppid.

Ah I might have found a thing to do on this lazy Saturday afternoon. At least until I go ice skating... :-)

[posborne]

It turns out I am blind. We are already using the libc getpid/getppid. More time for skating I guess.

[dhylands]

I'm happy to only expose gettid for linux and not for OSX, I just don't know how to do that on the rust side.

The SYS_gettid constant is different for different platforms (i.e. arm, 32-bit, and 64-bit x86 all have different constants for SYS_gettid) otherwise I could have done the syscall from rust. Maybe there's a way to create rust constants?

[dhylands]

And @kamalmarhubi is correct, glibc doesn't have a geittid function to expose, which is why the syscall needs to be done directly. Personally, I find it exremely useful to have the tid exposed as shows up in the ps listing (i.e. ps -T shows both pids and tids) and pthread_self would not be useful in this situation. Or for examining /proc/PID/task/TID.

[kamalmarhubi]

@dhylands

The SYS_gettid constant is different for different platforms (i.e. arm, 32-bit, and 64-bit x86 all have different constants for SYS_gettid) otherwise I could have done the syscall from rust. Maybe there's a way to create rust constants?

The right thing to do would be put them in the libc hierarchy, one of the files in here:

• https://github.com/rust-lang/libc/tree/master/src/unix/notbsd/linux/other/b32
• https://github.com/rust-lang/libc/tree/master/src/unix/notbsd/linux/other/b64

[kamalmarhubi]

@dhylands

I'm happy to only expose gettid for linux and not for OSX, I just don't know how to do that on the rust side.

The pattern is something like this:

#[cfg(target_os = "linux")]
pub fn gettid() -> pid_t {
   ...
}

Though here you probably want #[cfg(any(target_os = "linux", target_os = "android"))] for this.

[dhylands]

Ahh - ok - I'll have a go at adding SYS_gettid in libc then.

[kamalmarhubi]

@dhylands 👍

[homu]

☔ The latest upstream changes (presumably #294) made this pull request unmergeable. Please resolve the merge conflicts.

[dhylands]

I just got SYS_gettid merged into rust-lang/libc (rust-lang/libc#209), so I need to rework this anyways, since I can now do a rust-only solution.

[kamalmarhubi]

👍 :-)

[posborne]

Well done 👍

[dhylands]

I've updated the PR, but it can't be merged until libc releases 0.2.8 (and then I'll need to update the Cargo.toml file as well). It also looks like I got the type wrong on SYS_gettid, so I put through a libc PR to fix that: rust-lang/libc#213

I can build nix-rust locally (by redirecting libc to my local copy).

I had a question about the "as i32" and whether I should use as i32 or as pid_t. I'll guess I should change it to pid_t but I thoght I would find out what others thoughts were.

[dhylands]

Updated PR to use libc 0.2.8 and the one test that doesn't pass seems unrelated (it's for darwin and this PR should only affect linux). The same build on my repository was green:
https://travis-ci.org/dhylands/nix/builds/114422104

This PR adds a gettid by doing a syscall using libc::SYS_gettid

[kamalmarhubi]

@dhylands thanks! You're right, test_sigwait is flaky. I'll make a PR in a second to disable it, as it fails a bit too often...

[kamalmarhubi]

could you make this #[cfg(any(target_os = "linux", target_os = "android"))]?

[kamalmarhubi]

Just a minor change to the conditional compilation, and this is good to go! Thanks!

[dhylands]

ok - fixed up the conditionals. I had to make a few other changes as the tests started to fail in other mysterious ways, so I followed a pattern I found elsewhere in the test files.

The only failure is the test_semwait on darwin.

[kamalmarhubi]

Made the test retry and it works. @homu r+

[homu]

📌 Commit ca75212 has been approved by kamalmarhubi

[kamalmarhubi]

Thanks @dhylands!!

[homu]

⚡ Test exempted - status

[kamalmarhubi]

@dhylands

I had to make a few other changes as the tests started to fail in other mysterious ways, so I followed a pattern I found elsewhere in the test files.

What do you mean by this? Sounds like something we should document if it wasn't clear.

[dhylands]

Of course, I tried to go back and reproduce the problem and can't any more.

I was seeing local failures (when running cargo test locally) related to wait and execvpe. It was probably something stale in my build or tree or something.

[posborne]

The wait and execvpe problems are related to thread parallelism and should not happen with RUST_TEST_THREADS=1. Unfortunately, I am not aware of a way to set this up so when a developer runs cargo test things are automatically set up this way.

[kamalmarhubi]

I've got some ideas for fixing the tests (#251) which I'll try and do this week...

[posborne]

@kamalmarhubi Excellent, look forward to seeing what you come up with. As an aside, I put out a feeler in a reddit thread for suggestions on testing out systems code in rust without a good mocking framework: https://www.reddit.com/r/rust/comments/494tkf/looking_for_suggestions_mocking_external_functions/

Calling the actual system calls only lets us test the happy path (and even then not always as consistently as we would like).

[kamalmarhubi]

Moving conversation to #251