Re: [Bug-gnulib] addition: fatal-signal.h, fatal-signal.c

[Bruno Haible]

Paul Eggert wrote:

> >   - Removing temporary files. Here I don't call atexit() because if
> >     the program exits (due to a fatal error from the subprocess) the
> >     user needs to be able to look at the temporary file.
> ...
> It sounds like you want to remove temporary files in some cases, but
> not others.

Yes exactly.

> One way to do that is to register the cleanup function
> only in the cases where you want to remove temporary files.  Another
> way is to have the cleanup function inspect variables that tell it
> what needs cleaning up.

I don't know in advance whether the program will terminate through
exit() or through a fatal signal... And when either of these occur,
they only call the cleanup handler, without arguments, and without
setting any variable.

> I meant only that SIGRTMIN..SIGRTMAX should be added to the
> list of signals in the comment, i.e. signals that cause the program to
> exit but which the code doesn't catch for one reason or another.

OK, I see. I've added it as a comment now.

> > Do you really want a cleanup function that does a different thing
> > for SIGHUP than for SIGTERM?
>
> Sure.  I might want the cleanup function to print the name of the
> signal that killed it.  I might want even more details: if the
> implementation supports POSIX 1003.1-2001, I might want to use the
> siginfo_t information to print extra information associated with the
> signal, ...

Then you're out of the scope of the 'fatal-signal' module. A simple
facility for doing simple things, and the powerful sigaction() for
the rest.

I've made these modifications. Thanks for your feedback.


2003-10-14  Bruno Haible  <address@hidden>

        * m4/sig_atomic_t: New file, from GNU gettext.
        * m4/fatal-signal.m4 (gl_FATAL_SIGNAL): Require gt_TYPE_SIG_ATOMIC_T.
        * lib/fatal-signal.h: Improved comments. Suggested by Paul Eggert.
        * lib/fatal-signal.c: Use sig_atomic_t. Suggested by Paul Eggert.
        Also use volatile where needed.
        * modules/fatal-signal: Add m4/sig_atomic_t.m4 to file list.

============================= m4/sig_atomic_t.m4 ===========================
# sig_atomic_t.m4 serial 1 (gettext-0.12.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
dnl Public License, this file may be distributed as part of a program
dnl that contains a configuration script generated by Autoconf, under
dnl the same distribution terms as the rest of that program.

AC_DEFUN([gt_TYPE_SIG_ATOMIC_T],
[
  AC_CHECK_TYPES(sig_atomic_t, ,
    [AC_DEFINE(sig_atomic_t, int,
       [Define as an integer type suitable for memory locations that can be
        accessed atomically even in the presence of asynchnonous signals.])],
    [#include <signal.h>])
])
============================================================================
*** m4/fatal-signal.m4  6 Oct 2003 12:50:12 -0000       1.1
--- m4/fatal-signal.m4  14 Oct 2003 12:09:14 -0000      1.2
***************
*** 1,4 ****
! # fatal-signal.m4 serial 1
  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
--- 1,4 ----
! # 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,14 ****
--- 9,15 ----
  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)
  ])
diff -c -3 -r1.1 -r1.2
*** lib/fatal-signal.h  6 Oct 2003 12:50:11 -0000       1.1
--- lib/fatal-signal.h  14 Oct 2003 12:09:15 -0000      1.2
***************
*** 29,35 ****
     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.  */
  extern void at_fatal_signal (void (*function) (void));
  
  
--- 29,52 ----
     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.
! 
!    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,46 ****
     functions create the temporary file or directory _before_ returning its
     name to the application.  */
  
! /* Temporarily delay the catchable fatal signals.  */
  extern void block_fatal_signals (void);
  
  /* Stop delaying the catchable fatal signals.  */
--- 57,66 ----
     functions create the temporary file or directory _before_ returning its
     name to the application.  */
  
! /* 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.  */
*** lib/fatal-signal.c  6 Oct 2003 12:50:11 -0000       1.1
--- lib/fatal-signal.c  14 Oct 2003 12:09:15 -0000      1.2
***************
*** 50,55 ****
--- 50,56 ----
       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,95 ****
  /* ========================================================================= 
*/
  
  
- /* 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;
  static size_t actions_allocated = SIZEOF (static_actions);
  
  
--- 86,106 ----
  /* ========================================================================= 
*/
  
  
  typedef void (*action_t) (void);
! 
! /* 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,128 ****
        break;
        n--;
        actions_count = n;
!       action = actions[n];
        /* Execute the action.  */
        action ();
      }
  
!   /* Now execute the signal's default action.  */
    uninstall_handlers ();
  #if HAVE_RAISE
    raise (sig);
--- 128,142 ----
        break;
        n--;
        actions_count = n;
!       action = actions[n].action;
        /* Execute the action.  */
        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,178 ****
        /* 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;
        size_t new_actions_allocated = 2 * actions_allocated;
!       action_t *new_actions =
!       xmalloc (new_actions_allocated * sizeof (action_t));
  
!       memcpy (new_actions, actions, actions_allocated * sizeof (action_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;
    actions_count++;
  }
  
--- 174,197 ----
        /* Extend the actions array.  Note that we cannot use xrealloc(),
         because then the cleanup() function could access an already
         deallocated array.  */
!       actions_entry_t *old_actions = actions;
        size_t new_actions_allocated = 2 * actions_allocated;
!       actions_entry_t *new_actions =
!       xmalloc (new_actions_allocated * sizeof (actions_entry_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);
      }
!   /* 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,205 ****
--- 219,225 ----
      }
  }
  
+ /* Temporarily delay the catchable fatal signals.  */
  void
  block_fatal_signals ()
  {
***************
*** 207,212 ****
--- 227,233 ----
    sigprocmask (SIG_BLOCK, &fatal_signal_set, NULL);
  }
  
+ /* Stop delaying the catchable fatal signals.  */
  void
  unblock_fatal_signals ()
  {
*** modules/fatal-signal        6 Oct 2003 12:50:12 -0000       1.1
--- modules/fatal-signal        14 Oct 2003 12:09:15 -0000      1.2
***************
*** 6,11 ****
--- 6,12 ----
  lib/fatal-signal.c
  m4/fatal-signal.m4
  m4/signalblocking.m4
+ m4/sig_atomic_t.m4
  
  Depends-on:
  xalloc