Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it okay to use libc::__errno_location with rayon? #1069

Open
safinaskar opened this issue Jul 3, 2023 · 4 comments
Open

Is it okay to use libc::__errno_location with rayon? #1069

safinaskar opened this issue Jul 3, 2023 · 4 comments

Comments

@safinaskar
Copy link

In my codebase I run map on rayon's parallel iterator, then inside map's closure I call libc::linkat and then I read errno like so:

let iter = /* some rayon's parallel iterator */
let a = iter.map(|x|{
  ....
  if unsafe { libc::linkat(...) } == -1 {
    let err = unsafe { *libc::__errno_location() };
    ...
  }
  ...
});
/* use "a" */

Is this okay?

https://docs.rs/rayon/1.7.0/rayon/fn.scope.html says:

The closure given to scope() executes in the Rayon thread-pool, as do those given to spawn(). This means that you can’t access thread-local variables (well, you can, but they may have unexpected values).

This paragraph implies that in all threads executed in Rayon thread-pool one cannot access thread-local variables. And map execute its argument in Rayon thread-pool. And errno (as well as I know) is thread-local variable. So, is it okay to access errno in this code?

@adamreichold
Copy link
Collaborator

I think this is okay because you do nothing to yield into the thread pool between calling linkat and accessing errno.

(Doing anything that yields control to Rayon might end up calling other code which messes with the thread-local variables. Similarly, each invocation of your closure could happen on a different thread of the pool and hence cannot communicate via thread-local storage.)

@cuviper
Copy link
Member

cuviper commented Jul 3, 2023

Rayon doesn't move your closure once it's already running -- couldn't do that even if we wanted to, as you could have any kind of !Send data in your locals for which we have no visibility.

The point we're trying to make in those docs is that the closures that you pass to scope and spawn may execute on a different thread than you started from, and TLS doesn't move with them. But they're locked in place once they're called.

@safinaskar
Copy link
Author

It is possible to create a thread very low-level way, so that thread-local storage doesn't work at all. For example, using clone syscall in Linux with flag CLONE_THREAD. Current rayon's documentation text can be interpreted as "We create threads by directly executing clone(...CLONE_THREAD...) instead of any kind of high-level abstraction, so you are not allowed to use thread-local storage at all, otherwise you will get immediate undefined behavior (UB)".

Please, reword the docs such way that it will be clear that this is wrong interpretation. Write something like this: "Thread-local storage works. It is not UB to read it or write to it. It is okay to write to it and then immediately read from it. Just keep in mind that you cannot know where rayon will execute your code".

Current text says "but they may have unexpected values". I can interpret this as std::mem::uninitialized-style UB ( https://doc.rust-lang.org/nightly/std/mem/fn.uninitialized.html )

@cuviper
Copy link
Member

cuviper commented Jul 4, 2023

I can interpret this as std::mem::uninitialized-style UB

Certainly not -- it would be a major library unsoundness if rayon allowed UB from safe user code.

But I agree that "unexpected" is not clear, especially because that makes an assumption about what the user might be expecting. The TLS value may be different than the one where scope/spawn started from, if the threadpool ran it on a different thread, and to some users that may be perfectly intuitive.

ThreadPool::install and ThreadPool::broadcast have a little better note:

    /// # Warning: thread-local data
    ///
    /// Because `op` is executing within the Rayon thread-pool,
    /// thread-local data from the current thread will not be
    /// accessible.

But that's almost too strong -- you could end up executing on the same thread, if you call from within the pool in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants