Improved 'fatal-signal' module.
authorBruno Haible <bruno@clisp.org>
Tue, 14 Oct 2003 12:09:12 +0000 (12:09 +0000)
committerBruno Haible <bruno@clisp.org>
Tue, 14 Oct 2003 12:09:12 +0000 (12:09 +0000)
ChangeLog
lib/ChangeLog
lib/fatal-signal.c
lib/fatal-signal.h
m4/ChangeLog
m4/fatal-signal.m4
modules/fatal-signal

index 558b81e..d5d3f38 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2003-10-14  Bruno Haible  <bruno@clisp.org>
+
+       * modules/fatal-signal: Add m4/sig_atomic_t.m4 to file list.
+
 2003-10-12  Paul Eggert  <eggert@twinsun.com>
 
        * modules/xalloc: Do not depend on 'exit'.  Depend on 'stdbool'.
index 44630d5..c543f10 100644 (file)
@@ -1,3 +1,9 @@
+2003-10-14  Bruno Haible  <bruno@clisp.org>
+
+       * fatal-signal.h: Improved comments. Suggested by Paul Eggert.
+       * fatal-signal.c: Use sig_atomic_t. Suggested by Paul Eggert.
+       Also use volatile where needed.
+
 2003-10-12  Paul Eggert  <eggert@twinsun.com>
 
        * lib/xalloc.h (xnmalloc, xzalloc, xnrealloc, xclone): New decls.
index ea78012..8718fd6 100644 (file)
@@ -50,6 +50,7 @@
      SIGSTKFLT - because it is more similar to SIGFPE, SIGSEGV, SIGBUS,
      SIGSYS - because it is more similar to SIGABRT, SIGSEGV,
      SIGPWR - because it of too special use,
+     SIGRTMIN...SIGRTMAX - because they are reserved for application use.
    plus
      SIGXCPU, SIGXFSZ - because they are quite similar to SIGTERM.  */
 
@@ -85,11 +86,21 @@ static const int fatal_signals[] =
 /* ========================================================================= */
 
 
-/* The registered cleanup actions.  */
 typedef void (*action_t) (void);
-static action_t static_actions[32];
-static action_t * volatile actions = static_actions;
-static size_t volatile actions_count = 0;
+
+/* Type of an entry in the actions array.
+   The 'action' field is accessed from within the fatal_signal_handler(),
+   therefore we mark it as 'volatile'.  */
+typedef struct
+{
+  volatile action_t action;
+}
+actions_entry_t;
+
+/* The registered cleanup actions.  */
+static actions_entry_t static_actions[32];
+static actions_entry_t * volatile actions = static_actions;
+static sig_atomic_t volatile actions_count = 0;
 static size_t actions_allocated = SIZEOF (static_actions);
 
 
@@ -117,12 +128,15 @@ fatal_signal_handler (int sig)
        break;
       n--;
       actions_count = n;
-      action = actions[n];
+      action = actions[n].action;
       /* Execute the action.  */
       action ();
     }
 
-  /* Now execute the signal's default action.  */
+  /* Now execute the signal's default action.
+     If signal() blocks the signal being delivered for the duration of the
+     signal handler's execution, the re-raised signal is delivered when this
+     handler returns; otherwise it is delivered already during raise().  */
   uninstall_handlers ();
 #if HAVE_RAISE
   raise (sig);
@@ -160,19 +174,24 @@ at_fatal_signal (action_t action)
       /* Extend the actions array.  Note that we cannot use xrealloc(),
         because then the cleanup() function could access an already
         deallocated array.  */
-      action_t *old_actions = actions;
+      actions_entry_t *old_actions = actions;
       size_t new_actions_allocated = 2 * actions_allocated;
-      action_t *new_actions =
-       xmalloc (new_actions_allocated * sizeof (action_t));
+      actions_entry_t *new_actions =
+       xmalloc (new_actions_allocated * sizeof (actions_entry_t));
 
-      memcpy (new_actions, actions, actions_allocated * sizeof (action_t));
+      memcpy (new_actions, old_actions,
+             actions_allocated * sizeof (actions_entry_t));
       actions = new_actions;
       actions_allocated = new_actions_allocated;
       /* Now we can free the old actions array.  */
       if (old_actions != static_actions)
        free (old_actions);
     }
-  actions[actions_count] = action;
+  /* The two uses of 'volatile' in the types above (and ISO C 99 section
+     5.1.2.3.(5)) ensure that we increment the actions_count only after
+     the new action has been written to the memory location
+     actions[actions_count].  */
+  actions[actions_count].action = action;
   actions_count++;
 }
 
@@ -200,6 +219,7 @@ init_fatal_signal_set ()
     }
 }
 
+/* Temporarily delay the catchable fatal signals.  */
 void
 block_fatal_signals ()
 {
@@ -207,6 +227,7 @@ block_fatal_signals ()
   sigprocmask (SIG_BLOCK, &fatal_signal_set, NULL);
 }
 
+/* Stop delaying the catchable fatal signals.  */
 void
 unblock_fatal_signals ()
 {
index ffbc4a8..16e4f72 100644 (file)
@@ -29,7 +29,24 @@ extern "C" {
    The limitation of this facility is that it cannot work for SIGKILL.  */
 
 /* Register a cleanup function to be executed when a catchable fatal signal
-   occurs.  */
+   occurs.
+
+   Restrictions for the cleanup function:
+     - The cleanup function can do all kinds of system calls.
+     - It can also access application dependent memory locations and data
+       structures provided they are in a consistent state. One way to ensure
+       this is through block_fatal_signals()/unblock_fatal_signals(), see
+       below.  Another - more tricky - way to ensure this is the careful use
+       of 'volatile'.
+   However,
+     - malloc() and similarly complex facilities are not safe to be called
+       because they are not guaranteed to be in a consistent state.
+     - Also, the cleanup function must not block the catchable fatal signals
+       and leave them blocked upon return.
+
+   The cleanup function is executed asynchronously.  It is unspecified
+   whether during its execution the catchable fatal signals are blocked
+   or not.  */
 extern void at_fatal_signal (void (*function) (void));
 
 
@@ -40,7 +57,10 @@ extern void at_fatal_signal (void (*function) (void));
    functions create the temporary file or directory _before_ returning its
    name to the application.  */
 
-/* Temporarily delay the catchable fatal signals.  */
+/* Temporarily delay the catchable fatal signals.
+   The signals will be blocked (= delayed) until the next call to
+   unblock_fatal_signals().  If the signals are already blocked, a further
+   call to block_fatal_signals() has no effect.  */
 extern void block_fatal_signals (void);
 
 /* Stop delaying the catchable fatal signals.  */
index 443297a..8a27acf 100644 (file)
@@ -1,3 +1,8 @@
+2003-10-14  Bruno Haible  <bruno@clisp.org>
+
+       * sig_atomic_t: New file, from GNU gettext.
+       * fatal-signal.m4 (gl_FATAL_SIGNAL): Require gt_TYPE_SIG_ATOMIC_T.
+
 2003-10-12  Paul Eggert  <eggert@twinsun.com>
 
        * xalloc.m4 (gl_PREREQ_XMALLOC): Require AC_C_INLINE.
index 6d48e3a..c5d51e0 100644 (file)
@@ -1,4 +1,4 @@
-# fatal-signal.m4 serial 1
+# fatal-signal.m4 serial 2
 dnl Copyright (C) 2003 Free Software Foundation, Inc.
 dnl This file is free software, distributed under the terms of the GNU
 dnl General Public License.  As a special exception to the GNU General
@@ -9,6 +9,7 @@ dnl the same distribution terms as the rest of that program.
 AC_DEFUN([gl_FATAL_SIGNAL],
 [
   AC_REQUIRE([gt_SIGNALBLOCKING])
+  AC_REQUIRE([gt_TYPE_SIG_ATOMIC_T])
   AC_CHECK_HEADERS_ONCE(unistd.h)
   AC_CHECK_FUNCS(raise)
 ])
index e1a0715..fde4227 100644 (file)
@@ -6,6 +6,7 @@ lib/fatal-signal.h
 lib/fatal-signal.c
 m4/fatal-signal.m4
 m4/signalblocking.m4
+m4/sig_atomic_t.m4
 
 Depends-on:
 xalloc