On Wed, 21 Jan 2009, David Schultz wrote:

I think there *is* a real bug here, but there's two distinct ways
to fix it. When a threaded process forks, malloc acquires all its
locks so that its state is consistent after a fork. However, the
post-fork hook that's supposed to release these locks fails to do
so in the child because the child process isn't threaded, and
malloc_mutex_unlock() is optimized to be a no-op in
single-threaded processes. If the child *stays* single-threaded,
malloc() works by accident even with all the locks held because
malloc_mutex_lock() is also a no-op in single-threaded processes.
But if the child goes multi-threaded, then things break.

Solution 1 is to actually unlock the locks in the child process,
which is what Brian is proposing.

Solution 2 is to take the position that all of this pre- and
post-fork bloat in the fork() path is gratuitous and should be
removed. The rationale here is that if you fork with multiple
running threads, there's scads of ways in which the child's heap
could be inconsistent; fork hooks would be needed not just in
malloc(), but in stdio, third party libraries, etc. Why should
malloc() be special? It's the programmer's job to quiesce all the
threads before calling fork(), and if the programmer doesn't do
this, then POSIX only guarantees that async-signal-safe functions
will work.

Note that Solution 2 also fixes Brian's problem if he quiesces all
of his worker threads before forking (as he should!) With the
pre-fork hook removed, all the locks will start out free in the
child.  So that's what I vote for...

The problem is that our own libraries (libthr included)
need to malloc() for themselves, even after a fork() in
the child.  After a fork(), the malloc locks should be
reinitialized in the child if it was threaded, so that
our implementation actually works for all the async
signal calls, fork(), exec(), etc.  I forget the exact
failure modes for very common cases, but if you remove
the re-initialization of the malloc locks, I'm sure
you will have problems.

Perhaps much of this malloc() stuff goes away when we
move to pthread locks that are not pointers to allocated
objects, but instead are actual objects/structures.
This needs to be done in order for mutexes/CVs/etc
to be PTHREAD_PROCESS_SHARED (placed in shared memory
and used by multiple processes).  In other words,
pthread_mutex_t goes from this:

        typedef struct pthread_mutex *pthread_mutex_t;

to something like this:

        struct __pthread_mutex {
                uint32_t        lock;
                ...
        }
        typedef struct __pthread_mutex pthread_mutex_t;

Same thing for CVs, and we probably should convert any other
locks used internally by libc/libpthread (spinlocks).

So after a fork(), there is no need to reallocate anything,
it can just be reinitialized if necessary.

--
DE
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"

Reply via email to