HadrienG2 / hwlocality

Rust bindings to Open MPI Portable Hardware Locality "hwloc" library, covering version 2.0 and above.
MIT License
24 stars 6 forks source link

Not working on Alpine Linux (libc::pthread_t != std::os::linux::raw::pthread_t) #114

Closed HoKim98 closed 9 months ago

HoKim98 commented 9 months ago

On Alpine Linux, pthread_t type is mismatched between libc and std.

Error reconstruction

docker run --rm -it alpine:3.19  # Digest: x86_64 => sha256:6457d53fb065d6f250e1504b9bc42d5b6c65941d57532c072d929dd0628977d0

# In the docker container, we'll do the rest
apk add cargo git hwloc-dev eudev-dev

git clone "https://github.com/HadrienG2/hwlocality.git"
cd hwlocality
cargo test --all  # Error!

Logs

   Compiling hwlocality-sys v0.4.0 (/hwlocality/hwlocality-sys)
   Compiling hwlocality v1.0.0-alpha.1 (/hwlocality)
error[E0277]: `*mut c_void` doesn't implement `std::fmt::Display`
   --> src/cpu/binding.rs:805:62
    |
805 |             Self::Thread(id) => format!("the thread with TID {id}"),
    |                                                              ^^^^ `*mut c_void` cannot be formatted with the default formatter
    |
    = help: the trait `std::fmt::Display` is not implemented for `*mut c_void`
    = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
    = note: this error originates in the macro `$crate::__export::format_args` which comes from the expansion of the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
   --> src/lib.rs:288:9
    |
286 |     fn as_thread_id(&self) -> ThreadId {
    |                               -------- expected `*mut c_void` because of return type
287 |         use std::os::unix::thread::JoinHandleExt;
288 |         self.as_pthread_t()
    |         ^^^^^^^^^^^^^^^^^^^ expected `*mut c_void`, found `u64`
    |
    = note: expected raw pointer `*mut c_void`
                      found type `u64`

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `hwlocality` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error[E0277]: `*mut libc::c_void` doesn't implement `std::fmt::Display`
   --> src/cpu/binding.rs:805:62
    |
805 |             Self::Thread(id) => format!("the thread with TID {id}"),
    |                                                              ^^^^ `*mut libc::c_void` cannot be formatted with the default formatter
    |
    = help: the trait `std::fmt::Display` is not implemented for `*mut libc::c_void`
    = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
    = note: this error originates in the macro `$crate::__export::format_args` which comes from the expansion of the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
   --> src/lib.rs:288:9
    |
286 |     fn as_thread_id(&self) -> ThreadId {
    |                               -------- expected `*mut libc::c_void` because of return type
287 |         use std::os::unix::thread::JoinHandleExt;
288 |         self.as_pthread_t()
    |         ^^^^^^^^^^^^^^^^^^^ expected `*mut c_void`, found `u64`
    |
    = note: expected raw pointer `*mut libc::c_void`
                      found type `u64`

error: could not compile `hwlocality` (lib test) due to 2 previous errors

Expected Solution

If we cast the type ThreadId to *mut libc::c_void, then all examples would be failed because ThreadId is not thread-safe.

So I think the plausible option is: type ThreadId = u64.

HadrienG2 commented 9 months ago

Do we know why libc::pthread_t is void* on Alpine ? Is it a general musl thing ?

HadrienG2 commented 9 months ago

I have cross-checked the musl source code, and it turns out that their pthread_t definition is made of as much insanity as the rest of their threading support...

#ifdef __cplusplus
TYPEDEF unsigned long pthread_t;
#else
TYPEDEF struct __pthread * pthread_t;
#endif

...so I guess I'll just redefine ThreadId to be c_ulong on musl and call it a day. :shrug:

HoKim98 commented 9 months ago

Wow, musl is so amazing. I've never seen such an inconvenient and insane implementation.

HadrienG2 commented 9 months ago

Try to update to the freshly released hwlocality-sys v0.4.1 and you should hopefully be good to go.

HoKim98 commented 9 months ago

Sorry for necrobumping, but there is one remaining error

error[E0308]: mismatched types
   --> src/lib.rs:266:14
    |
264 | pub fn current_thread_id() -> ThreadId {
    |                               -------- expected `u64` because of return type
265 |     // SAFETY: Should be always safe to call
266 |     unsafe { libc::pthread_self() }
    |              ^^^^^^^^^^^^^^^^^^^^ expected `u64`, found `*mut c_void`
    |
    = note:     expected type `u64`
            found raw pointer `*mut c_void`

For more information about this error, try `rustc --explain E0308`.
error: could not compile `hwlocality` (lib) due to previous error
warning: build failed, waiting for other jobs to finish...
HadrienG2 commented 9 months ago

This should be resolved by #116, and the hwlocality 1.0.0-alpha.2 release that I'll push which integrates it.

Long-term, I should set up a musl build in CI to avoid this sort of issues, but I do not have the time for this right now.

HadrienG2 commented 9 months ago

Should be good now.