Closed GoogleCodeExporter closed 9 years ago
GoogleCodeExporter
commented
9 years ago I afraid we can't do anything here.
mixing fork and threads is not supposed to work even w/o asan,
you can find many poofs of that in the net.
Sorry. Original comment by konstant...@gmail.com on 28 Dec 2013 at 9:18
GoogleCodeExporter
commented
9 years ago Mixing fork and threads is clearly tricky, this is not forbidden and have some
working use cases (in my case, snapshotting the state of a database)... you
mostly have to make sure that the forked thread don't use mutexes from the
parent process.
I'll find a workaround of my own, but I really think this is a regression from
ASan since it worked perfectly from day 1 and only broke when using clang 3.4.Original comment by florent....@intersec.com on 28 Dec 2013 at 10:03
GoogleCodeExporter
commented
9 years ago http://cppwisdom.quora.com/Why-threads-and-fork-dont-mixOriginal comment by konstant...@gmail.com on 28 Dec 2013 at 5:06
GoogleCodeExporter
commented
9 years ago I've attached a patch that fix my issue (only on Linux, in a quick and dirty
fashion). The idea is to have a registry of AsanThreads. When a thread is
created or destroyed, the registry is updated. After forking, in the child
process, we scan the registry in order to destroy all the threads that didn't
survive the fork. The patch depends on pthread_atfork() to perform the cleanup.
The registry implementation is naïve and limited. And only linux is
implemented and tested. Also I didn't include a test.
I still perfectly agree that mixing fork and threads in the general case is
dangerous. However, I also still believe that my use case is perfectly valid: I
use fork() as a way to take a snapshot of the memory of my database in order to
be able to make a snapshot while still accepting updates in the parent process
(I don't think there is a better solution for this). The threads are not
strictly required in the child process, however they are mandatory in the
parent process. In my use case the child process only writes data that is in
memory onto the disk, since it works on a snapshot of the memory it does not
need synchronization (so all the argument about critical sections don't apply).
Hope this can help.Original comment by florent....@intersec.com on 28 Dec 2013 at 9:54
Attachments:
GoogleCodeExporter
commented
9 years ago I am skeptical about anything that involves pthread_atfork --
from what I've heard this thing is broken beyond repair and only seems to work
sometimes.
Still, if you provide a test case and a proper patch we may consider it,
please see https://code.google.com/p/address-sanitizer/wiki/HowToContributeOriginal comment by konstant...@gmail.com on 9 Jan 2014 at 7:58
GoogleCodeExporter
commented
9 years ago A comment from dvyukov@ that didn't make it into the bug tracker:
========================================
> I'll find a workaround of my own, but I really think this is a regression
> from ASan since it worked perfectly from day 1 and only broke when using
> clang 3.4.
Yes, strictly saying it is legal.
POSIX says:
"A process shall be created with a single thread. If a multi-threaded
process calls fork(), the new process shall contain a replica of the
calling thread and its entire address space, possibly including the
states of mutexes and other resources. Consequently, to avoid errors,
the child process may only execute async-signal-safe operations until
such time as one of the exec functions is called. [THR] Fork
handlers may be established by means of thepthread_atfork() function
in order to maintain application invariants across fork() calls."
http://pubs.opengroup.org/onlinepubs/009695399/functions/fork.html
It's not even required to execute only async-signal safe functions. If
you are careful to establish invariants, you can use any functions.
========================================
I'm afraid we'll finally need to intercept fork() (instead of using
pthread_atfork()) and at least clean up the thread registry in
sanitizer_common, because other tools may need it.
As we've seen recently, even in the case fork() is followed by exec() (which is
totally legitimate) we should be really careful to avoid deadlocks in the
tools' guts.
Original comment by ramosian.glider@gmail.com on 10 Jan 2014 at 2:39
GoogleCodeExporter
commented
9 years ago > I still perfectly agree that mixing fork and threads in the general case is
> dangerous. However, I also still believe that my use case is perfectly valid:
I use
> fork() as a way to take a snapshot of the memory of my database in order to
be able
> to make a snapshot while still accepting updates in the parent process (I
don't
> think there is a better solution for this).
Side note: this is how process snapshotting works in Breakpad (an error
reporting system used by Chromium). I don't think we want to prohibit this
pattern.Original comment by ramosian.glider@gmail.com on 10 Jan 2014 at 2:41
Original issue reported on code.google.com by
florent....@intersec.comon 27 Dec 2013 at 9:38