[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<BY5PR07MB7126E731A452CE0234873F30C1F02@BY5PR07MB7126.namprd07.prod.outlook.com>
Date: Sat, 8 Feb 2025 05:25:30 +0000
From: "Zhu, Yifan" <yzhu104@...Rochester.edu>
To: Rich Felker <dalias@...c.org>, "libc-coord@...ts.openwall.com"
<libc-coord@...ts.openwall.com>
CC: Matthew Parkinson <mattpark@...rosoft.com>
Subject: Re: [EXT] Re: pthread_atfork concurrent semantics
> However there may be some valid motivation to
allow concurrent forks.
I don't think this is a situation for concurrent fork. Rather, it is a situation where fork and atfork executes concurrently.
> FWIW this has UB because the child attempts to unlock a mutex it does not own.
Indeed, the mutex operation may not be well-defined. However, if didn't understand it wrong, mutex is just a "concept" here. Userspace spinlock and other routines with exclusivity may also run into the problem as long as the exclusivity status resides in private maps. snmalloc allocator is vendoring its own synchronization primitives from what I can tell.
> Can you clarify what kind of synchronization you have in mind whereby
these events are distinguishably concurrent?
The problem snmalloc had was that if fork and malloc/free are executed concurrently, there are chances that certain locks are held across forks. Although malloc/free are not async-signal-free, support for fork may still be interesting:
1. snmalloc can be used separately from POSIX malloc/free, so there may be situations users what to use it as a cross-fork allocator (though I think such situations should be rare).
2. In general, avoiding deadlock is a good thing to do. I think FreeBSD and bionic do have special cross-fork hooks to deal with their malloc/free. It would be great if we can get some inputs from FreeBSD libc and bionic.
Best,
Yifan
Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Rich Felker <dalias@...c.org>
Sent: Saturday, February 8, 2025 12:16:23 AM
To: libc-coord@...ts.openwall.com <libc-coord@...ts.openwall.com>
Cc: Zhu, Yifan <yzhu104@...Rochester.edu>; Matthew Parkinson <mattpark@...rosoft.com>
Subject: [EXT] Re: [libc-coord] pthread_atfork concurrent semantics
On Wed, Feb 05, 2025 at 09:47:25AM +0000, Matthew Parkinson wrote:
> Hi all,
>
> I have been looking at using pthread_atfork to make snmalloc fork
> safe, however, I am unsure on the semantics in the concurrent
> setting, and there are differences in the implementations of libc.
> Yifan Zhu suggested I post the issue here.
>
> Consider the following scenario with two threads: one is performing
> a fork, and the other is performing a pthread_atfork. If
> pthread_atfork returns, should you be guaranteed that the forking
> thread will execute the atfork handler?
Can you clarify what kind of synchronization you have in mind whereby
these events are distinguishably concurrent? Your test program just
does unsynchronized sleeps, but I suspect you could make what you want
rigorous by putting actual synchronization actions in the atfork
handler.
FWIW the atfork handlers in the parent need to run in reverse order of
registration, so I think it's clear that no newly added handler can be
run after the old first handler has already run. Likewise, the
post-fork handlers cannot run (in parent or child) if the
corresponding pre-fork one was not run.
> I want this kind of pattern to use a Singleton pattern to install
> the pthread_atfork on first use, but that requires that
> pthread_atfork returning to ensure that the handler being install in
> parallel with a fork, then the fork will use the handler.
>
> I have a small example here:
> ```
> std::mutex m;
>
> void prefork_for_m() {
> m.lock();
> }
>
> void postfork_for_m() {
> m.unlock();
> }
>
> void slow_prefork()
> {
> // Make prefork take a long time to encourage a race
> // with a call to pthread_atfork
> std::this_thread::sleep_for(std::chrono::milliseconds(500));
> }
>
> int main()
> {
> std::thread([]{
> // Sleep to ensure the first handler has been registered.
> std::this_thread::sleep_for(std::chrono::milliseconds(200));
>
> pthread_atfork(prefork_for_m, postfork_for_m, postfork_for_m); // (a)
>
> m.lock();
> //sleep to allow fork to complete while we are here.
> std::this_thread::sleep_for(std::chrono::milliseconds(1000));
>
> m.unlock();
> }).detach();
>
> pthread_atfork(slow_prefork, nullptr, nullptr); // (b)
>
> pid_t pid = fork(); // (c)
>
> m.lock(); // (d)
> m.unlock();
>
> std::this_thread::sleep_for(std::chrono::milliseconds(1000));
>
> if (pid != 0)
> {
> wait(nullptr);
> }
>
> return 0;
> }
> ```
FWIW this has UB because the child attempts to unlock a mutex it does
not own. POSIX noted this fundamental flaw. It should be avoidable by
using semaphores rather than mutexes though. Arguably it may also be
avoidable by re-initializing the mutex in the child instead of
attempting to unlock it.
> This example on recent glibc will deadlock the child process at
> (d). This is because the fork at (c) will execute the handler from
> (b), and due to the fix "Bug 27054 - pthread_atfork handlers that
> call pthread_atfork deadlock"
> (https://urldefense.com/v3/__https://sourceware.org/bugzilla/show_bug.cgi?id=27054__;!!CGUSO5OYRnA7CQ!baP1aeGLXDoRb1ru3nNvuqAeeN0J-ZIZ0v-6FxcAjT_ONm3m4rAsSmOMKxGPtlgvVFhecYPT4xWIii_JGSYrcLE$ ), this will
> drop the lock protecting the list. This enables the other thread to
> install a handler (a), but this is ignored by the fork.
>
> Musl and earlier versions of glibc would prevent the child
> deadlocking as they hold a lock protecting the list for the entire
> duration of the fork.
>
> I have filled an issue on glibc
> (https://urldefense.com/v3/__https://sourceware.org/bugzilla/show_bug.cgi?id=32615__;!!CGUSO5OYRnA7CQ!baP1aeGLXDoRb1ru3nNvuqAeeN0J-ZIZ0v-6FxcAjT_ONm3m4rAsSmOMKxGPtlgvVFhecYPT4xWIii_JztyRlDg$ ), but it is
> unclear to me if this is a bug, or allowed behaviour.
I think glibc is in the wrong letting you add new atfork handlers
while fork is executing them in another thread, since, based on the
above analysis, I don't think there's any way to get the right locking
behavior if that's done. However there may be some valid motivation to
allow concurrent forks. That would be achievable with the lock being
rwlock-like with fork as reader and atfork as writer. This does not
solve the problem glibc #27054 was trying to solve, but I think the
motivation for that report/change was wrong/misguided and I've
commented on it as such.
Rich
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
