fork(): Defer signals and lock the heaps while _kern_fork().

* Also defer signals while registering fork hooks.
* While malloc provides fork heap hooks which lock the heaps and unlock/reinit,
  malloc_debug provides empty hooks.
* Ideas suggested by Ingo, patch reviewed by him. Thanks a lot!
* Also call fork parent hooks on failure.
* Solve locks-up when combining multithreading and process forking, should help
  with #13111.
This commit is contained in:
Jérôme Duval
2016-12-17 21:11:15 +01:00
parent 194afffd85
commit 17b2a3cfcb
8 changed files with 101 additions and 20 deletions
@@ -36,6 +36,9 @@ void __init_env(const struct user_space_program_args *args);
void __init_env_post_heap(void);
status_t __init_heap(void);
void __heap_terminate_after(void);
void __heap_before_fork(void);
void __heap_after_fork_child(void);
void __heap_after_fork_parent(void);
void __init_time(addr_t commPageTable);
void __arch_init_time(struct real_time_data *data, bool setDefaults);
@@ -67,8 +67,8 @@ static size_t sFreeHeapSize, sHeapAreaSize;
static free_chunk *sFreeChunks;
static void
init_after_fork(void)
void
__init_after_fork(void)
{
// find the heap area
sHeapArea = area_for((void*)sFreeHeapBase);
@@ -110,13 +110,6 @@ __init_heap(void)
hoardLockInit(sHeapLock, "heap");
atfork(&init_after_fork);
// Note: Needs malloc(). Hence we need to be fully initialized.
// TODO: We should actually also install a hook that is called before
// fork() is being executed. In a multithreaded app it would need to
// acquire *all* allocator locks, so that we don't fork() an
// inconsistent state.
return B_OK;
}
+1 -2
View File
@@ -132,8 +132,7 @@ hoardHeap::hoardHeap(void)
, _magic(HEAP_MAGIC)
#endif
{
// Initialize the per-heap lock.
hoardLockInit(_lock, "hoard heap");
initLock();
for (int i = 0; i < SUPERBLOCK_FULLNESS_GROUP; i++) {
for (int j = 0; j < SIZE_CLASSES; j++) {
+11
View File
@@ -115,6 +115,9 @@ class hoardHeap {
// Unlock this heap.
inline void unlock(void);
// Init this heap lock.
inline void initLock(void);
// Set our index number (which heap we are).
inline void setIndex(int i);
@@ -444,6 +447,14 @@ hoardHeap::unlock(void)
}
void
hoardHeap::initLock(void)
{
// Initialize the per-heap lock.
hoardLockInit(_lock, "hoard heap");
}
size_t
hoardHeap::align(const size_t sz)
{
@@ -67,6 +67,9 @@ class processHeap : public hoardHeap {
// Get a thread heap index.
inline int getHeapIndex(void);
// Get thread heap max.
inline int getMaxThreadHeaps(void);
// Get the thread heap with index i.
inline HEAPTYPE & getHeap(int i);
@@ -199,6 +202,15 @@ processHeap::getHeapIndex(void)
}
// Return the maximum number of heaps.
int
processHeap::getMaxThreadHeaps(void)
{
return fMaxThreadHeaps;
}
superblock *
processHeap::acquire(const int sizeclass, hoardHeap * dest)
{
@@ -256,6 +256,35 @@ getAllocator(void)
}
extern "C" void
__heap_before_fork(void)
{
static processHeap *pHeap = getAllocator();
for (int i = 0; i < pHeap->getMaxThreadHeaps(); i++)
pHeap->getHeap(i).lock();
}
void __init_after_fork(void);
extern "C" void
__heap_after_fork_child(void)
{
__init_after_fork();
static processHeap *pHeap = getAllocator();
for (int i = 0; i < pHeap->getMaxThreadHeaps(); i++)
pHeap->getHeap(i).initLock();
}
extern "C" void
__heap_after_fork_parent(void)
{
static processHeap *pHeap = getAllocator();
for (int i = 0; i < pHeap->getMaxThreadHeaps(); i++)
pHeap->getHeap(i).unlock();
}
// #pragma mark - public functions
@@ -202,6 +202,24 @@ __heap_terminate_after()
}
extern "C" void
__heap_before_fork(void)
{
}
extern "C" void
__heap_after_fork_child(void)
{
}
extern "C" void
__heap_after_fork_parent(void)
{
}
// #pragma mark - Public API
+25 -9
View File
@@ -16,6 +16,7 @@
#include <pthread_private.h>
#include <runtime_loader.h>
#include <syscalls.h>
#include <user_thread.h>
typedef struct fork_hook {
@@ -105,9 +106,13 @@ call_fork_hooks(fork_hook *hook)
status_t
__register_atfork(void (*prepare)(void), void (*parent)(void), void (*child)(void))
{
defer_signals();
status_t status = mutex_lock(&sForkLock);
if (status != B_OK)
if (status != B_OK) {
undefer_signals();
return status;
}
if (prepare)
status = add_fork_hook(&sPrepareHooks, NULL, prepare);
@@ -119,6 +124,9 @@ __register_atfork(void (*prepare)(void), void (*parent)(void), void (*child)(voi
status = add_fork_hook(&sChildHooks, &sLastChildHook, child);
mutex_unlock(&sForkLock);
undefer_signals();
return status;
}
@@ -129,21 +137,19 @@ fork(void)
thread_id thread;
status_t status;
defer_signals();
status = mutex_lock(&sForkLock);
if (status != B_OK)
if (status != B_OK) {
undefer_signals();
return status;
}
// call preparation hooks
call_fork_hooks(sPrepareHooks);
__heap_before_fork();
thread = _kern_fork();
if (thread < 0) {
// something went wrong
mutex_unlock(&sForkLock);
__set_errno(thread);
return -1;
}
if (thread == 0) {
// we are the child
// ToDo: initialize child
@@ -155,15 +161,25 @@ fork(void)
// process we should make sure that it is in a consistent state when
// calling the kernel.
__gRuntimeLoader->reinit_after_fork();
__heap_after_fork_child();
__reinit_pwd_backend_after_fork();
call_fork_hooks(sChildHooks);
} else {
// we are the parent
__heap_after_fork_parent();
call_fork_hooks(sParentHooks);
mutex_unlock(&sForkLock);
}
undefer_signals();
if (thread < 0) {
// something went wrong
__set_errno(thread);
thread = -1;
}
return thread;
}