[lttng-dev] [PATCH liburcu] Fix pthread_atfork() behaviour

[Mathieu Desnoyers]

----- Original Message -----
> From: "Keir Fraser" <keir at cohodata.com>
> To: lttng-dev at lists.lttng.org
> Sent: Monday, April 14, 2014 9:31:57 AM
> Subject: [lttng-dev] [PATCH liburcu] Fix pthread_atfork() behaviour
> 
> In the process of integrating liburcu into a multi-threaded codebase
> with fork()s I found a couple of problems with liburcu that I could not
> work around without fixing the library. Hence I present the two required
> fixes here (as attachments, sorry!) with some background info about them.

This is interesting!

An initial question: Which URCU flavor are you using ?

> 
> After fork() the child process has no pthreads but the one that called
> fork(). Unfortunately call_rcu_after_fork_child() does not update URCU's
> thread registry to reflect this -- if fork() is called with any threads
> registered with URCU then the child process will inherit a corrupted
> registry containing a linked list through per-thread TLS state which is
> no longer valid allocated memory.

You appear to be using the mb/memb URCU flavor.

If we look at URCU README file:

Interaction with fork()

        Special care must be taken for applications performing fork() without
        any following exec(). This is caused by the fact that Linux only clones
        the thread calling fork(), and thus never replicates any of the other
        parent thread into the child process. Most liburcu implementations
        require that all registrations (as reader, defer_rcu and call_rcu
        threads) should be released before a fork() is performed, except for the
        rather common scenario where fork() is immediately followed by exec() in
        the child process. The only implementation not subject to that rule is
        liburcu-bp, which is designed to handle fork() by calling
        rcu_bp_before_fork, rcu_bp_after_fork_parent and
        rcu_bp_after_fork_child.

        Applications that use call_rcu() and that fork() without
        doing an immediate exec() must take special action.  The parent
        must invoke call_rcu_before_fork() before the fork() and
        call_rcu_after_fork_parent() after the fork().  The child
        process must invoke call_rcu_after_fork_child().
        Even though these three APIs are suitable for passing to
        pthread_atfork(), use of pthread_atfork() is *STRONGLY
        DISCOURAGED* for programs calling the glibc memory allocator
        (malloc(), calloc(), free(), ...) within call_rcu callbacks.
        This is due to limitations in the way glibc memory allocator
        handles calls to the memory allocator from concurrent threads
        while the pthread_atfork() handlers are executing.
        Combining e.g.:
        * call to free() from callbacks executed within call_rcu worker
          threads,
        * executing call_rcu atfork handlers within the glibc pthread
          atfork mechanism,
        will sometimes trigger interesting process hangs. This usually
        hangs on a memory allocator lock within glibc.


> Crash or hang soon after is the
> result. Patch 1 therefore simply clears the registry list in the child
> process. Caveats here are that (a) the calling thread cannot be
> registered (it must unregister/re-register itself in the atfork
> handlers); and (b) some flavours of URCU may have more complex
> registries than a simple linked list and so this patch may not be
> sufficient for those. I only tested the memb/mb flavour myself.

Firstly, your use-case seems to be only supported by the urcu-bp
flavor, and explicitly unsupported by other flavors. If you want
to do this with other urcu flavors, you need to explicitly
unregister all your other RCU threads before doing the fork().

Also, if we want to eventually support this in the other flavors,
we would need to implement the flavor-specific teardown within each
flavor implementation rather than in the call_rcu implementation.

Moreover, as stated in the README file, call_rcu() pre/post fork
handlers don't behave well with glibc's pthread_atfork(), due to
assumptions done within the glibc memory allocator.

> 
> A second problem is that although call_rcu threads are paused across
> fork(), the handshaking PAUSED flag is not cleared when their execution
> resumes. Hence a second fork() invocation in the original parent process
> will not spin-wait for call_rcu threads to quiesce (as the atfork
> handler will observe all PAUSED flags already set). Patch 2 fixes this
> with the appropriate clearing handshake on resume, post-fork.

I'd be very interested to see a test-case for this problem against
the urcu-bp flavor. I think you have indeed caught a real bug here.

> 
> Please feel free to modify or rewrite these patches, or solve the
> described problems in a different way, as you see fit! Cc me on replies
> as I am not a subscriber to this list.

Could you provide a small test-case that we could add to the call_rcu
regression (double fork) under tests/regression to make sure this behavior
is covered in the future ? A small refactoring of test_urcu_fork.c to
extend it should do the trick.

Thanks!

Mathieu

> 
>   Regards,
>   Keir Fraser
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1-zap-thread-registry.patch
Type: text/x-patch
Size: 1043 bytes
Desc: not available
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20140417/58a7bbdf/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2-clear-paused-flag.patch
Type: text/x-patch
Size: 1487 bytes
Desc: not available
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20140417/58a7bbdf/attachment-0001.bin>