Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/lib/libpthread



On 31.01.2020 22:10, Andrew Doran wrote:
> On Fri, Jan 31, 2020 at 06:55:00PM -0000, Christos Zoulas wrote:
> 
>> In article <724af477-010b-9ddf-6ece-e23d7cf59079%gmx.com@localhost>,
>> Kamil Rytarowski  <n54%gmx.com@localhost> wrote:
>>> -=-=-=-=-=-
>>> -=-=-=-=-=-
>>>
>>> On 31.01.2020 03:38, Christos Zoulas wrote:
>>>> And it is fixed now.
>>>>
>>>> christos
>>>>
>>>
>>> OK. I am going to submit a bug report upstream and get some feedback
>>> what is the way forward here, delaying initialization.
>>
>> I think that the way forward (on our side) is to do away with libpthread,
>> merge it with libc and kill all the stub nonsense.
> 
> Agreed.
> 
> pthread__init() does some expensive stuff like _lwp_ctl().  I think we can
> safely & without hacks defer a lot of that till the first pthread_create().
> 
> Andrew
> 

This libc-libpthread split/merge is a red herring.

The problem here is with a mutual dependencies between POSIX threads
library and malloc library.

I did some investigation and here are my findings:

1. jemalloc abuses initialization and initializes self very early, with
a constructor:

/*
 * If an application creates a thread before doing any allocation in the
main
 * thread, then calls fork(2) in the main thread followed by memory
allocation
 * in the child process, a race can occur that results in deadlock
within the
 * child: the main thread may have forked while the created thread had
 * partially initialized the allocator.  Ordinarily jemalloc prevents
 * fork/malloc races via the following functions it registers during
 * initialization using pthread_atfork(), but of course that does no good if
 * the allocator isn't fully initialized at fork time.  The following
library
 * constructor is a partial solution to this problem.  It may still be
possible
 * to trigger the deadlock described above, but doing so would involve
forking
 * via a library constructor that runs before jemalloc's runs.
 */
#ifndef JEMALLOC_JET
JEMALLOC_ATTR(constructor)
static void

jemalloc_constructor(void) {
        malloc_init();
}
#endif

Relevant commit:

commit 20f1fc95adb35ea63dc61f47f2b0ffbd37d39f32
Author: Jason Evans <je%fb.com@localhost>
Date:   Tue Oct 9 14:46:22 2012 -0700

    Fix fork(2)-related deadlocks.

    Add a library constructor for jemalloc that initializes the allocator.
    This fixes a race that could occur if threads were created by the main
    thread prior to any memory allocation, followed by fork(2), and then
    memory allocation in the child process.

    Fix the prefork/postfork functions to acquire/release the ctl, prof, and
    rtree mutexes.  This fixes various fork() child process deadlocks, but
    one possible deadlock remains (intentionally) unaddressed: prof
    backtracing can acquire runtime library mutexes, so deadlock is still
    possible if heap profiling is enabled during fork().  This deadlock is
    known to be a real issue in at least the case of libgcc-based
    backtracing.

    Reported by tfengjun.

2. FreeBSD added a hack and an internal pthread_mutex_init() version
called: _pthread_mutex_init_calloc_cb().. it passes a callback pointer
to jemalloc's tiny calloc().

This is very ugly and I consider it as the wrong way of boostraping malloc.

3. There is a problem inside libpthread. It as designed to not malloc()
early to not trigger malloc initialization, however it was broken as it
calls at_fork functions:

#0  malloc (size=size@entry=16) at
/usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:2052
#1  0x0000776b71d71a44 in af_alloc () at
/usr/src/lib/libc/gen/pthread_atfork.c:80
#2  af_alloc () at /usr/src/lib/libc/gen/pthread_atfork.c:74
#3  _pthread_atfork (prepare=prepare@entry=0x0, parent=parent@entry=0x0,
    child=child@entry=0x776b7220b3e5 <pthread__fork_callback>)
    at /usr/src/lib/libc/gen/pthread_atfork.c:121
#4  0x0000776b7220cacd in pthread__init () at
/usr/src/lib/libpthread/pthread.c:260
#5  0x0000776b71d7a585 in _libc_init () at
/usr/src/lib/libc/misc/initfini.c:128

These at_fork routines caused also issues with false-positives in Leak
Sanitizer. I had to pacify the sanitizer and disable tracking of its
allocations.


This patch removes '#if 0' hack from src/lib/libpthread and switches
at_fork to mmap()+munmap().

http://netbsd.org/~kamil/patch-00219-libpthread-libc-jemalloc.txt

This test disabled the constructor hack:

http://netbsd.org/~kamil/patch-00220-jemalloc-disable-constructor.txt

With these changes everything seems to work.

In order to avoid the FreeBSD specific hack with the constructor and
initialize jemalloc always during libc bootstrap I propose the following
approach:

 - add __libc_malloc_init() call in _libc_init()
 - redirect __libc_malloc_init() to jemalloc__init() with jemalloc
 - otherwise redirect to an empty stub

Here is a patch that does everything and works fine for me.

http://netbsd.org/~kamil/patch-00222-jemalloc-enhancements.txt

There are no longer jemalloc calls before being ready and jemalloc is
still initialized always but in its proper time.

(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /tmp/a.out

Breakpoint 2, jemalloc__init () at
/usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:3209
3209            malloc_init();
(gdb) c
Continuing.

Breakpoint 3, malloc_init_hard () at
/usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:1533
1533    malloc_init_hard(void) {
(gdb) bt
#0  malloc_init_hard () at
/usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:1533
#1  0x00007f7ff763f9c4 in ?? () from /usr/lib/libc.so.12
#2  0x00007f7ff7ef9400 in ?? ()
#3  0x00007f7ff763ac99 in _init () from /usr/lib/libc.so.12
#4  0x0000000000000000 in ?? ()

Attachment: signature.asc
Description: OpenPGP digital signature



Home | Main Index | Thread Index | Old Index