theRockLiu / thread-sanitizer

Automatically exported from code.google.com/p/thread-sanitizer
0 stars 0 forks source link

Feature request: detect fork() without exec() #44

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
It took me a while to debug 
https://code.google.com/p/chromium/issues/detail?id=324489, where TSan 
deadlocked in a child process that was calling an async-signal-unsafe routine 
soon after calling fork() in a multi-threaded program.

It would be useful if TSan notified me about the child process taking locks 
after fork() if there've been threads prior to fork() (TSan's own background 
thread should be ignored).

Original issue reported on code.google.com by gli...@google.com on 2 Dec 2013 at 11:32

GoogleCodeExporter commented 9 years ago
How exactly should it work?
There can be [almost] correct programs that do this. E.g. if after fork we only 
lock mutexes that were not locked by other threads before fork. Or if before 
fork we ensure that the mutexes are not locked (by locking them).

Original comment by dvyu...@google.com on 9 Dec 2013 at 2:29

GoogleCodeExporter commented 9 years ago
I need TSan to help me quickly and correctly identify the situation when the 
program acquires a lock in one thread, then calls fork() in another thread and 
then tries to acquire the same lock in that thread in the child process.

In the case the lock belongs to TSan runtime (e.g. if this is the reporting 
lock):
1) TSan has to make sure this lock isn't acquired anymore. I'm guessing that 
most of this can be done by disabling race detection after fork() if there were 
multiple threads prior to fork(). However there may be single-threaded errors 
(e.g. UAF) which we won't be able to report then. Perhaps a better solution is 
to make sure all the relevant locks are released before calling fork()

In the case the lock belongs to the client program:
2) TSan can report any attempt to acquire a lock taken by a dead thread. This 
way we'll catch only those deadlocks that actually happen and won't report any 
false positives.
3) If fork() has been called in the presence of other threads, TSan can report 
any attempt to call an async signal unsafe function (even if it's not leading 
to actual errors). Even less conservative approach is to report thread creation 
after fork() if there were threads before (may lead to false positives, 
although doing so is still UB)

malloc() calls deserve special handling, because they may cause acquiring the 
same locks by the TSan runtime and the user code.
We can either force fork() to be synchronized with malloc() calls in other 
threads (e.g. take the malloc lock before forking), or detect races between 
unsynchronized fork() and malloc() calls.

Original comment by gli...@google.com on 9 Dec 2013 at 4:12

GoogleCodeExporter commented 9 years ago

Original comment by dvyu...@google.com on 9 Dec 2013 at 4:15

GoogleCodeExporter commented 9 years ago
Yet another thing that needs to be done is to restart the background thread 
upon fork() if there were no user thread before fork().
Otherwise the child will have the shadow memory garbage collection silently 
disabled.

Original comment by gli...@google.com on 30 Dec 2013 at 12:01

GoogleCodeExporter commented 9 years ago
Fixed by r199993.

The background thread is recreated.
Tsan will detect multi-threaded fork followed by creation of new threads in the 
child process.

Original comment by dvyu...@google.com on 24 Jan 2014 at 12:45