Skip to content

Unix process spawn may trigger UB for pthread lock behavior. #82879

Closed
@ehuss

Description

@ehuss
Contributor

When spawning a process in the case where posix_spawn cannot be used, the spawn code uses fork/execvp. It acquires the environment lock before the fork, with a drop handler that unlocks it. Unfortunately, this is done in both the parent and child processes, and unlocking a lock acquired from another thread is undefined behavior (see pthread_rwlock_unlock and pthread_mutex_unlock).

An example of where this can happen is rustdoc running doctests. It has N threads all spawning rustc processes. It was observed in #82221 that this was frequently causing deadlocks (i686, on a Docker image with glibc 2.23).

PR #82877 reverts the change from mutex to rwlock, but pthread_mutex_unlock is also undefined behavior, we just fortunately have not run into any problems. This should be fixed. This can probably be done by mem::forget'ing the guard.

https://stackoverflow.com/questions/61976745/why-does-rust-rwlock-behave-unexpectedly-with-fork also provides some insight into why unlocking a rwlock after a fork doesn't work.

rustc 1.52.0-nightly (caca212 2021-03-05)

Activity

joshtriplett

joshtriplett commented on Mar 8, 2021

@joshtriplett
Member

Based on some discussion on Zulip, I think the right approach would be to reinitialize the lock in the child, immediately after the fork.

We can't just forget the lock; that will break usage of the environment within a pre_exec function. Reinitializing the lock in the child should work.

ghost

ghost commented on Mar 8, 2021

@ghost

Is this a duplicate of #64718?

(Also, as mentioned in #64718 (comment), it seems none of the exec*p* functions is signal-safe:)

$ man signal-safety | rg -F exec
       trant or because it is atomic with respect to signals (i.e., its execu‐
       execl(3)               Added in POSIX.1-2008; see notes below
       execle(3)              See notes below
       execv(3)               Added in POSIX.1-2008
       execve(2)
       fexecve(3)             Added in POSIX.1-2008
       *  If a signal handler interrupts the execution of an unsafe  function,
       *  Before glibc 2.24, execl(3) and execle(3) employed realloc(3) inter
sfackler

sfackler commented on Mar 8, 2021

@sfackler
Member

We can't just forget the lock; that will break usage of the environment within a pre_exec function. Reinitializing the lock in the child should work.

But getenv and setenv are not async-signal-safe, so isn't usage of the environment after a multi-threaded fork inherently broken?

ehuss

ehuss commented on Mar 8, 2021

@ehuss
ContributorAuthor

Indeed this is a duplicate of #64718, thanks for the link!

Closing, let's keep the discussion consolidated there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: This is a bug.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @ehuss@joshtriplett@sfackler

        Issue actions

          Unix process spawn may trigger UB for pthread lock behavior. · Issue #82879 · rust-lang/rust