diff options
Diffstat (limited to 'malloc')
| -rw-r--r-- | malloc/Makefile | 3 | ||||
| -rw-r--r-- | malloc/arena.c | 58 | ||||
| -rw-r--r-- | malloc/malloc-internal.h | 32 | ||||
| -rw-r--r-- | malloc/malloc.c | 1 | ||||
| -rw-r--r-- | malloc/tst-malloc-fork-deadlock.c | 220 |
5 files changed, 272 insertions, 42 deletions
diff --git a/malloc/Makefile b/malloc/Makefile index 59d4264ae0..3283f4f75f 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -29,7 +29,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-malloc-usable tst-realloc tst-posix_memalign \ tst-pvalloc tst-memalign tst-mallopt tst-scratch_buffer \ tst-malloc-backtrace tst-malloc-thread-exit \ - tst-malloc-thread-fail + tst-malloc-thread-fail tst-malloc-fork-deadlock test-srcs = tst-mtrace routines = malloc morecore mcheck mtrace obstack \ @@ -49,6 +49,7 @@ libmemusage-inhibit-o = $(filter-out .os,$(object-suffixes)) $(objpfx)tst-malloc-backtrace: $(shared-thread-library) $(objpfx)tst-malloc-thread-exit: $(shared-thread-library) $(objpfx)tst-malloc-thread-fail: $(shared-thread-library) +$(objpfx)tst-malloc-fork-deadlock: $(shared-thread-library) # These should be removed by `make clean'. extra-objs = mcheck-init.o libmcheck.a diff --git a/malloc/arena.c b/malloc/arena.c index cd26cdd02c..bba83e9360 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -133,10 +133,6 @@ static void *(*save_malloc_hook)(size_t __size, const void *); static void (*save_free_hook) (void *__ptr, const void *); static void *save_arena; -# ifdef ATFORK_MEM -ATFORK_MEM; -# endif - /* Magic value for the thread-specific arena pointer when malloc_atfork() is in use. */ @@ -202,14 +198,14 @@ free_atfork (void *mem, const void *caller) /* Counter for number of times the list is locked by the same thread. */ static unsigned int atfork_recursive_cntr; -/* The following two functions are registered via thread_atfork() to - make sure that the mutexes remain in a consistent state in the - fork()ed version of a thread. Also adapt the malloc and free hooks - temporarily, because the `atfork' handler mechanism may use - malloc/free internally (e.g. in LinuxThreads). */ +/* The following three functions are called around fork from a + multi-threaded process. We do not use the general fork handler + mechanism to make sure that our handlers are the last ones being + called, so that other fork handlers can use the malloc + subsystem. */ -static void -ptmalloc_lock_all (void) +void +__malloc_fork_lock_parent (void) { mstate ar_ptr; @@ -217,7 +213,7 @@ ptmalloc_lock_all (void) return; /* We do not acquire free_list_lock here because we completely - reconstruct free_list in ptmalloc_unlock_all2. */ + reconstruct free_list in __malloc_fork_unlock_child. */ if (mutex_trylock (&list_lock)) { @@ -242,7 +238,7 @@ ptmalloc_lock_all (void) __free_hook = free_atfork; /* Only the current thread may perform malloc/free calls now. save_arena will be reattached to the current thread, in - ptmalloc_lock_all, so save_arena->attached_threads is not + __malloc_fork_lock_parent, so save_arena->attached_threads is not updated. */ save_arena = thread_arena; thread_arena = ATFORK_ARENA_PTR; @@ -250,8 +246,8 @@ out: ++atfork_recursive_cntr; } -static void -ptmalloc_unlock_all (void) +void +__malloc_fork_unlock_parent (void) { mstate ar_ptr; @@ -262,8 +258,8 @@ ptmalloc_unlock_all (void) return; /* Replace ATFORK_ARENA_PTR with save_arena. - save_arena->attached_threads was not changed in ptmalloc_lock_all - and is still correct. */ + save_arena->attached_threads was not changed in + __malloc_fork_lock_parent and is still correct. */ thread_arena = save_arena; __malloc_hook = save_malloc_hook; __free_hook = save_free_hook; @@ -277,15 +273,8 @@ ptmalloc_unlock_all (void) (void) mutex_unlock (&list_lock); } -# ifdef __linux__ - -/* In NPTL, unlocking a mutex in the child process after a - fork() is currently unsafe, whereas re-initializing it is safe and - does not leak resources. Therefore, a special atfork handler is - installed for the child. */ - -static void -ptmalloc_unlock_all2 (void) +void +__malloc_fork_unlock_child (void) { mstate ar_ptr; @@ -321,11 +310,6 @@ ptmalloc_unlock_all2 (void) atfork_recursive_cntr = 0; } -# else - -# define ptmalloc_unlock_all2 ptmalloc_unlock_all -# endif - /* Initialization routine. */ #include <string.h> extern char **_environ; @@ -394,7 +378,6 @@ ptmalloc_init (void) #endif thread_arena = &main_arena; - thread_atfork (ptmalloc_lock_all, ptmalloc_unlock_all, ptmalloc_unlock_all2); const char *s = NULL; if (__glibc_likely (_environ != NULL)) { @@ -469,14 +452,6 @@ ptmalloc_init (void) __malloc_initialized = 1; } -/* There are platforms (e.g. Hurd) with a link-time hook mechanism. */ -#ifdef thread_atfork_static -thread_atfork_static (ptmalloc_lock_all, ptmalloc_unlock_all, \ - ptmalloc_unlock_all2) -#endif - - - /* Managing heaps and arenas (for concurrent threads) */ #if MALLOC_DEBUG > 1 @@ -822,7 +797,8 @@ _int_new_arena (size_t size) limit is reached). At this point, some arena has to be attached to two threads. We could acquire the arena lock before list_lock to make it less likely that reused_arena picks this new arena, - but this could result in a deadlock with ptmalloc_lock_all. */ + but this could result in a deadlock with + __malloc_fork_lock_parent. */ (void) mutex_lock (&a->mutex); diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h new file mode 100644 index 0000000000..b830d3f58f --- /dev/null +++ b/malloc/malloc-internal.h @@ -0,0 +1,32 @@ +/* Internal declarations for malloc, for use within libc. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public License as + published by the Free Software Foundation; either version 2.1 of the + License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; see the file COPYING.LIB. If + not, see <http://www.gnu.org/licenses/>. */ + +#ifndef _MALLOC_PRIVATE_H +#define _MALLOC_PRIVATE_H + +/* Called in the parent process before a fork. */ +void __malloc_fork_lock_parent (void) internal_function attribute_hidden; + +/* Called in the parent process after a fork. */ +void __malloc_fork_unlock_parent (void) internal_function attribute_hidden; + +/* Called in the child process after a fork. */ +void __malloc_fork_unlock_child (void) internal_function attribute_hidden; + + +#endif /* _MALLOC_PRIVATE_H */ diff --git a/malloc/malloc.c b/malloc/malloc.c index 1eed79414c..7aad75a26d 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -244,6 +244,7 @@ /* For ALIGN_UP et. al. */ #include <libc-internal.h> +#include <malloc/malloc-internal.h> /* Debugging: diff --git a/malloc/tst-malloc-fork-deadlock.c b/malloc/tst-malloc-fork-deadlock.c new file mode 100644 index 0000000000..94549ca459 --- /dev/null +++ b/malloc/tst-malloc-fork-deadlock.c @@ -0,0 +1,220 @@ +/* Test concurrent fork, getline, and fflush (NULL). + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public License as + published by the Free Software Foundation; either version 2.1 of the + License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; see the file COPYING.LIB. If + not, see <http://www.gnu.org/licenses/>. */ + +#include <sys/wait.h> +#include <unistd.h> +#include <errno.h> +#include <stdio.h> +#include <pthread.h> +#include <stdbool.h> +#include <stdlib.h> +#include <malloc.h> +#include <time.h> +#include <string.h> +#include <signal.h> + +static int do_test (void); +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" + +enum { + /* Number of threads which call fork. */ + fork_thread_count = 4, + /* Number of threads which call getline (and, indirectly, + malloc). */ + read_thread_count = 8, +}; + +static bool termination_requested; + +static void * +fork_thread_function (void *closure) +{ + while (!__atomic_load_n (&termination_requested, __ATOMIC_RELAXED)) + { + pid_t pid = fork (); + if (pid < 0) + { + printf ("error: fork: %m\n"); + abort (); + } + else if (pid == 0) + _exit (17); + + int status; + if (waitpid (pid, &status, 0) < 0) + { + printf ("error: waitpid: %m\n"); + abort (); + } + if (!WIFEXITED (status) || WEXITSTATUS (status) != 17) + { + printf ("error: waitpid returned invalid status: %d\n", status); + abort (); + } + } + return NULL; +} + +static char *file_to_read; + +static void * +read_thread_function (void *closure) +{ + FILE *f = fopen (file_to_read, "r"); + if (f == NULL) + { + printf ("error: fopen (%s): %m\n", file_to_read); + abort (); + } + + while (!__atomic_load_n (&termination_requested, __ATOMIC_RELAXED)) + { + rewind (f); + char *line = NULL; + size_t line_allocated = 0; + ssize_t ret = getline (&line, &line_allocated, f); + if (ret < 0) + { + printf ("error: getline: %m\n"); + abort (); + } + free (line); + } + fclose (f); + + return NULL; +} + +static void * +flushall_thread_function (void *closure) +{ + while (!__atomic_load_n (&termination_requested, __ATOMIC_RELAXED)) + if (fflush (NULL) != 0) + { + printf ("error: fflush (NULL): %m\n"); + abort (); + } + return NULL; +} + +static void +create_threads (pthread_t *threads, size_t count, void *(*func) (void *)) +{ + for (size_t i = 0; i < count; ++i) + { + int ret = pthread_create (threads + i, NULL, func, NULL); + if (ret != 0) + { + errno = ret; + printf ("error: pthread_create: %m\n"); + abort (); + } + } +} + +static void +join_threads (pthread_t *threads, size_t count) +{ + for (size_t i = 0; i < count; ++i) + { + int ret = pthread_join (threads[i], NULL); + if (ret != 0) + { + errno = ret; + printf ("error: pthread_join: %m\n"); + abort (); + } + } +} + +/* Create a file which consists of a single long line, and assigns + file_to_read. The hope is that this triggers an allocation in + getline which needs a lock. */ +static void +create_file_with_large_line (void) +{ + int fd = create_temp_file ("bug19431-large-line", &file_to_read); + if (fd < 0) + { + printf ("error: create_temp_file: %m\n"); + abort (); + } + FILE *f = fdopen (fd, "w+"); + if (f == NULL) + { + printf ("error: fdopen: %m\n"); + abort (); + } + for (int i = 0; i < 50000; ++i) + fputc ('x', f); + fputc ('\n', f); + if (ferror (f)) + { + printf ("error: fputc: %m\n"); + abort (); + } + if (fclose (f) != 0) + { + printf ("error: fclose: %m\n"); + abort (); + } +} + +static int +do_test (void) +{ + /* Make sure that we do not exceed the arena limit with the number + of threads we configured. */ + if (mallopt (M_ARENA_MAX, 400) == 0) + { + printf ("error: mallopt (M_ARENA_MAX) failed\n"); + return 1; + } + + /* Leave some room for shutting down all threads gracefully. */ + int timeout = 3; + if (timeout > TIMEOUT) + timeout = TIMEOUT - 1; + + create_file_with_large_line (); + + pthread_t fork_threads[fork_thread_count]; + create_threads (fork_threads, fork_thread_count, fork_thread_function); + pthread_t read_threads[read_thread_count]; + create_threads (read_threads, read_thread_count, read_thread_function); + pthread_t flushall_threads[1]; + create_threads (flushall_threads, 1, flushall_thread_function); + + struct timespec ts = {timeout, 0}; + if (nanosleep (&ts, NULL)) + { + printf ("error: error: nanosleep: %m\n"); + abort (); + } + + __atomic_store_n (&termination_requested, true, __ATOMIC_RELAXED); + + join_threads (flushall_threads, 1); + join_threads (read_threads, read_thread_count); + join_threads (fork_threads, fork_thread_count); + + free (file_to_read); + + return 0; +} |