This is the mail archive of the glibc-cvs@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

GNU C Library master sources branch master updated. glibc-2.23-314-g56290d6


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  56290d6e762c1194547e73ff0b948cd79d3a1e03 (commit)
      from  cd065b68439076971640047cc6eaada27dcfd0f7 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=56290d6e762c1194547e73ff0b948cd79d3a1e03

commit 56290d6e762c1194547e73ff0b948cd79d3a1e03
Author: Florian Weimer <fweimer@redhat.com>
Date:   Thu May 12 08:54:17 2016 +0200

    Increase fork signal safety for single-threaded processes [BZ #19703]
    
    This provides a band-aid and addresses the scenario where fork is
    called from a signal handler while the process is in the malloc
    subsystem (or has acquired the libio list lock).  It does not
    address the general issue of async-signal-safety of fork;
    multi-threaded processes are not covered, and some glibc
    subsystems have fork handlers which are not async-signal-safe.

diff --git a/ChangeLog b/ChangeLog
index dee85ca..e623247 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2016-05-12  Florian Weimer  <fweimer@redhat.com>
 
+	[BZ #19703]
+	Partially async-signal-safe fork for single-threaded processes.
+	* sysdeps/nptl/fork.c (__libc_fork): Introduce multiple_threads
+	variable.  Do not acquire and reset/release malloc and libio locks
+	in single-threaded processes.
+	* malloc/tst-mallocfork2.c: New file.
+	* malloc/Makefile (tests): Add it.
+
+2016-05-12  Florian Weimer  <fweimer@redhat.com>
+
 	* sysdeps/posix/getaddrinfo.c (gaih_inet_serv): Add tmpbuf
 	argument.  Use scratch buffer instead of extend_alloca.
 	(gethosts): Use scratch buffer instead of extend_alloca.
diff --git a/malloc/Makefile b/malloc/Makefile
index 3283f4f..fa1730e 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -29,7 +29,8 @@ 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-fork-deadlock
+	 tst-malloc-thread-fail tst-malloc-fork-deadlock \
+	 tst-mallocfork2
 test-srcs = tst-mtrace
 
 routines = malloc morecore mcheck mtrace obstack \
diff --git a/malloc/tst-mallocfork2.c b/malloc/tst-mallocfork2.c
new file mode 100644
index 0000000..a9e3e94
--- /dev/null
+++ b/malloc/tst-mallocfork2.c
@@ -0,0 +1,212 @@
+/* Test case for async-signal-safe fork (with respect to malloc).
+   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/>.  */
+
+/* This test will fail if the process is multi-threaded because we
+   only have an async-signal-safe fork in the single-threaded case
+   (where we skip acquiring the malloc heap locks).
+
+   This test only checks async-signal-safety with regards to malloc;
+   other, more rarely-used glibc subsystems could have locks which
+   still make fork unsafe, even in single-threaded processes.  */
+
+#include <errno.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/wait.h>
+#include <time.h>
+#include <unistd.h>
+
+/* How many malloc objects to keep arond.  */
+enum { malloc_objects = 1009 };
+
+/* The maximum size of an object.  */
+enum { malloc_maximum_size = 70000 };
+
+/* How many signals need to be delivered before the test exits.  */
+enum { signal_count = 1000 };
+
+
+/* Process ID of the subprocess which sends SIGUSR1 signals.  */
+static pid_t sigusr1_sender_pid;
+
+/* Set to 1 if SIGUSR1 is received.  Used to detect a signal during
+   malloc/free.  */
+static volatile sig_atomic_t sigusr1_received;
+
+/* Periodically set to 1, to indicate that the process is making
+   progress.  Checked by liveness_signal_handler.  */
+static volatile sig_atomic_t progress_indicator = 1;
+
+/* Write the message to standard output.  Usable from signal
+   handlers.  */
+static void
+write_message (const char *str)
+{
+  write (STDOUT_FILENO, str, strlen (str));
+}
+
+static void
+sigusr1_handler (int signo)
+{
+  /* Let the main program make progress, by temporarily suspending
+     signals from the subprocess.  */
+  if (sigusr1_received)
+    return;
+  if (kill (sigusr1_sender_pid, SIGSTOP) != 0)
+    {
+      write_message ("error: kill (SIGSTOP)\n");
+      abort ();
+    }
+  sigusr1_received = 1;
+
+  /* Perform a fork with a trivial subprocess.  */
+  pid_t pid = fork ();
+  if (pid == -1)
+    {
+      write_message ("error: fork\n");
+      abort ();
+    }
+  if (pid == 0)
+    _exit (0);
+  int status;
+  int ret = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0));
+  if (ret < 0)
+    {
+      write_message ("error: waitpid\n");
+      abort ();
+    }
+  if (status != 0)
+    {
+      write_message ("error: unexpected exit status from subprocess\n");
+      abort ();
+    }
+}
+
+static void
+liveness_signal_handler (int signo)
+{
+  if (progress_indicator)
+    progress_indicator = 0;
+  else
+    write_message ("warning: process seems to be stuck\n");
+}
+
+static void
+__attribute__ ((noreturn))
+signal_sender (int signo, bool sleep)
+{
+  pid_t target = getppid ();
+  while (true)
+    {
+      if (kill (target, signo) != 0)
+        {
+          dprintf (STDOUT_FILENO, "error: kill: %m\n");
+          abort ();
+        }
+      if (sleep)
+        usleep (1 * 1000 * 1000);
+    }
+}
+
+static int
+do_test (void)
+{
+  struct sigaction action =
+    {
+      .sa_handler = sigusr1_handler,
+    };
+  sigemptyset (&action.sa_mask);
+
+  if (sigaction (SIGUSR1, &action, NULL) != 0)
+    {
+      printf ("error: sigaction: %m");
+      return 1;
+    }
+
+  action.sa_handler = liveness_signal_handler;
+  if (sigaction (SIGUSR2, &action, NULL) != 0)
+    {
+      printf ("error: sigaction: %m");
+      return 1;
+    }
+
+  pid_t sigusr2_sender_pid = fork ();
+  if (sigusr2_sender_pid == 0)
+    signal_sender (SIGUSR2, true);
+  sigusr1_sender_pid = fork ();
+  if (sigusr1_sender_pid == 0)
+    signal_sender (SIGUSR1, false);
+
+  void *objects[malloc_objects] = {};
+  unsigned signals = 0;
+  unsigned seed = 1;
+  time_t last_report = 0;
+  while (signals < signal_count)
+    {
+      progress_indicator = 1;
+      int slot = rand_r (&seed) % malloc_objects;
+      size_t size = rand_r (&seed) % malloc_maximum_size;
+      if (kill (sigusr1_sender_pid, SIGCONT) != 0)
+        {
+          printf ("error: kill (SIGCONT): %m\n");
+          signal (SIGUSR1, SIG_IGN);
+          kill (sigusr1_sender_pid, SIGKILL);
+          kill (sigusr2_sender_pid, SIGKILL);
+          return 1;
+        }
+      sigusr1_received = false;
+      free (objects[slot]);
+      objects[slot] = malloc (size);
+      if (sigusr1_received)
+        {
+          ++signals;
+          time_t current = time (0);
+          if (current != last_report)
+            {
+              printf ("info: SIGUSR1 signal count: %u\n", signals);
+              last_report = current;
+            }
+        }
+      if (objects[slot] == NULL)
+        {
+          printf ("error: malloc: %m\n");
+          signal (SIGUSR1, SIG_IGN);
+          kill (sigusr1_sender_pid, SIGKILL);
+          kill (sigusr2_sender_pid, SIGKILL);
+          return 1;
+        }
+    }
+
+  /* Clean up allocations.  */
+  for (int slot = 0; slot < malloc_objects; ++slot)
+    free (objects[slot]);
+
+  /* Terminate the signal-sending subprocess.  The SIGUSR1 handler
+     should no longer run because it uses sigusr1_sender_pid.  */
+  signal (SIGUSR1, SIG_IGN);
+  kill (sigusr1_sender_pid, SIGKILL);
+  kill (sigusr2_sender_pid, SIGKILL);
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
index 1a68cbd..616d897 100644
--- a/sysdeps/nptl/fork.c
+++ b/sysdeps/nptl/fork.c
@@ -54,6 +54,12 @@ __libc_fork (void)
     struct used_handler *next;
   } *allp = NULL;
 
+  /* Determine if we are running multiple threads.  We skip some fork
+     handlers in the single-thread case, to make fork safer to use in
+     signal handlers.  POSIX requires that fork is async-signal-safe,
+     but our current fork implementation is not.  */
+  bool multiple_threads = THREAD_GETMEM (THREAD_SELF, header.multiple_threads);
+
   /* Run all the registered preparation handlers.  In reverse order.
      While doing this we build up a list of all the entries.  */
   struct fork_handler *runp;
@@ -109,12 +115,21 @@ __libc_fork (void)
       break;
     }
 
-  _IO_list_lock ();
+  /* If we are not running multiple threads, we do not have to
+     preserve lock state.  If fork runs from a signal handler, only
+     async-signal-safe functions can be used in the child.  These data
+     structures are only used by unsafe functions, so their state does
+     not matter if fork was called from a signal handler.  */
+  if (multiple_threads)
+    {
+      _IO_list_lock ();
 
-  /* Acquire malloc locks.  This needs to come last because fork
-     handlers may use malloc, and the libio list lock has an indirect
-     malloc dependency as well (via the getdelim function).  */
-  __malloc_fork_lock_parent ();
+      /* Acquire malloc locks.  This needs to come last because fork
+	 handlers may use malloc, and the libio list lock has an
+	 indirect malloc dependency as well (via the getdelim
+	 function).  */
+      __malloc_fork_lock_parent ();
+    }
 
 #ifndef NDEBUG
   pid_t ppid = THREAD_GETMEM (THREAD_SELF, tid);
@@ -173,14 +188,18 @@ __libc_fork (void)
 # endif
 #endif
 
-      /* Release malloc locks.  */
-      __malloc_fork_unlock_child ();
+      /* Reset the lock state in the multi-threaded case.  */
+      if (multiple_threads)
+	{
+	  /* Release malloc locks.  */
+	  __malloc_fork_unlock_child ();
 
-      /* Reset the file list.  These are recursive mutexes.  */
-      fresetlockfiles ();
+	  /* Reset the file list.  These are recursive mutexes.  */
+	  fresetlockfiles ();
 
-      /* Reset locks in the I/O code.  */
-      _IO_list_resetlock ();
+	  /* Reset locks in the I/O code.  */
+	  _IO_list_resetlock ();
+	}
 
       /* Reset the lock the dynamic loader uses to protect its data.  */
       __rtld_lock_initialize (GL(dl_load_lock));
@@ -217,11 +236,15 @@ __libc_fork (void)
       /* Restore the PID value.  */
       THREAD_SETMEM (THREAD_SELF, pid, parentpid);
 
-      /* Release malloc locks, parent process variant.  */
-      __malloc_fork_unlock_parent ();
+      /* Release acquired locks in the multi-threaded case.  */
+      if (multiple_threads)
+	{
+	  /* Release malloc locks, parent process variant.  */
+	  __malloc_fork_unlock_parent ();
 
-      /* We execute this even if the 'fork' call failed.  */
-      _IO_list_unlock ();
+	  /* We execute this even if the 'fork' call failed.  */
+	  _IO_list_unlock ();
+	}
 
       /* Run the handlers registered for the parent.  */
       while (allp != NULL)

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                |   10 ++
 malloc/Makefile          |    3 +-
 malloc/tst-mallocfork2.c |  212 ++++++++++++++++++++++++++++++++++++++++++++++
 sysdeps/nptl/fork.c      |   53 ++++++++---
 4 files changed, 262 insertions(+), 16 deletions(-)
 create mode 100644 malloc/tst-mallocfork2.c


hooks/post-receive
-- 
GNU C Library master sources


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]