The use of the backtrace functions in the signal handler is unavoidable
(I think) for this feature to work. Although they are not listed as
safe, I think the risk of interference should be very low. And in any
case, this code will only execute when the service has already crashed,
so the worst thing that can happen is that the process hangs instead of
generating a core dump. The use of snprintf could be avoided by writing
our own code that does the same job (print the signal number as decimal
(or hexadecimal, which could be slightly easier), but I am not sure if
it is worth the effort. snprintf is likely to be implemented as
reentrant in glibc.
/ Anders Widell
On 04/09/2015 08:41 AM, ramesh betham wrote:
> Please don't use those functions (like syslog etc.) that are not
> /Async/-/signal/-/safe /functions in signal handler function.
>
> Thanks,
> Ramesh.
>
> On 4/8/2015 6:03 PM, Anders Widell wrote:
>> Ack with comments.
>>
>> * Make sure to have a properly formatted commit message ("base:" was
>> duplicated, square brackets missing around ticket number).
>> * It could be useful to see the identity of one who sent the signal
>> (PID, maybe also UID).
>> * See below for inline comments.
>>
>> / Anders Widell
>>
>> On 04/01/2015 10:32 AM, Hans Nordeback wrote:
>>> osaf/libs/core/common/daemon.c | 117
>>> +++++++++++++++++++++++++++++++++++++++++
>>> 1 files changed, 117 insertions(+), 0 deletions(-)
>>>
>>>
>>> Use dlsym on bactrace symbols to circumvent lsb compliance problems.
>>>
>>> diff --git a/osaf/libs/core/common/daemon.c
>>> b/osaf/libs/core/common/daemon.c
>>> --- a/osaf/libs/core/common/daemon.c
>>> +++ b/osaf/libs/core/common/daemon.c
>>> @@ -34,6 +34,8 @@
>>> #include <sys/stat.h>
>>> #include <sys/file.h>
>>> +#include <dlfcn.h>
>>> +
>>> #include <configmake.h>
>>> #include "daemon.h"
>>> @@ -43,6 +45,8 @@
>>> #include <os_defs.h>
>>> #include <osaf_secutil.h>
>>> +#include <sys/types.h>
>>> +#include <time.h>
>>> #define DEFAULT_RUNAS_USERNAME "opensaf"
>>> @@ -57,6 +61,8 @@ static unsigned int __tracemask;
>>> static unsigned int __nofork = 0;
>>> static int __logmask;
>>> +static void install_fatal_signal_handlers();
>> C functions taking no parameters should be declared as (void) instead
>> of ().
>>> +
>>> static void __print_usage(const char* progname, FILE* stream, int
>>> exit_code)
>>> {
>>> fprintf(stream, "Usage: %s [OPTIONS]...\n", progname);
>>> @@ -350,6 +356,9 @@ void daemonize(int argc, char *argv[])
>>> }
>>> #endif
>>> + /* install fatal signal handlers for writing backtrace to
>>> file */
>>> + install_fatal_signal_handlers();
>>> +
>> Whitespace problems on the three lines above (space instead of tab,
>> whitespace at end of line).
>>> /* Initialize the log/trace interface */
>>> if (logtrace_init_daemon(basename(argv[0]), __tracefile,
>>> __tracemask, __logmask) != 0)
>>> exit(EXIT_FAILURE);
>>> @@ -414,3 +423,111 @@ void daemon_sigterm_install(int *term_fd
>>> *term_fd = term_sel_obj.rmv_obj;
>>> }
>>> +
>>> +static char bt_filename[80];
>> This variable shall hold the result of sprintf(..., PKGLOGDIR
>> "/bt_%s_%d", ...). PKLOGDIR is a path that can be configured by the
>> user. Therefore, this buffer ought to be either PATH_MAX bytes large,
>> or dynamically allocated.
>>> +
>>> +static int (*plibc_backtrace) (void **buffer, int size) = NULL;
>>> +static int (*plibc_backtrace_symbols_fd) (void *const *buffer, int
>>> size, int fd) = NULL;
>>> +
>>> +static int init_backtrace_fptrs()
>> C functions taking no parameters should be declared as (void) instead
>> of ().
>>> +{
>>> + plibc_backtrace = dlsym(RTLD_NEXT, "backtrace");
>> Should use RTLD_DEFAULT instead of RTLD_NEXT.
>>> + if (plibc_backtrace == NULL) {
>>> + syslog(LOG_ERR, "unable to find \"backrace\" symbol");
>>> + return -1;
>>> + }
>>> + plibc_backtrace_symbols_fd = dlsym(RTLD_NEXT,
>>> "backtrace_symbols_fd");
>> Should use RTLD_DEFAULT instead of RTLD_NEXT.
>>> + if (plibc_backtrace_symbols_fd == NULL) {
>>> + syslog(LOG_ERR, "unable to find \"backrace_symbols_fd\"
>>> symbol");
>>> + return -1;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * Signal handler for fatal errors. Writes a backtrace and
>>> "re-throws" the signal.
>>> + */
>>> +static void fatal_signal_handler(int sig, siginfo_t* siginfo, void*
>>> ctx)
>>> +{
>>> + const int BT_ARRAY_SIZE = 20;
>>> + void *bt_array[BT_ARRAY_SIZE];
>>> + size_t bt_size;
>>> + int fd;
>>> + char bt_header[30];
>>> +
>>> + if ((fd = open(bt_filename, O_RDWR|O_CREAT, 0644)) < 0) {
>>> + goto done;
>>> + }
>>> +
>>> + snprintf(bt_header, sizeof(bt_header), "signal: %d\n", sig);
>>> +
>>> + if (write(fd, bt_header, strlen(bt_header)) < 0) {
>>> + close(fd);
>>> + goto done;
>>> + }
>>> +
>>> + bt_size = plibc_backtrace(bt_array, BT_ARRAY_SIZE);
>>> + plibc_backtrace_symbols_fd(bt_array, bt_size, fd);
>>> +
>>> + close(fd);
>>> +done:
>>> + // re-throw the signal
>>> + raise(sig);
>>> +}
>>> +
>>> +/**
>>> + * Install signal handlers for fatal errors to be able to print a
>>> backtrace.
>>> + */
>>> +static void install_fatal_signal_handlers()
>> C functions taking no parameters should be declared as (void) instead
>> of ().
>>> +{
>>> +
>>> + time_t current_time;
>>> + char time_string[20];
>>> +
>>> + struct sigaction action;
>>> + int i = 0;
>>> + const int HANDLED_SIGNALS_MAX = 7;
>>> + static const int handled_signals[] = {
>>> + SIGHUP,
>>> + SIGILL,
>>> + SIGABRT,
>>> + SIGFPE,
>>> + SIGSEGV,
>>> + SIGPIPE,
>>> + SIGBUS
>>> + };
>>> +
>>> + // to circumvent lsb use dlsym to retrieve backtrace in runtime
>>> + if (init_backtrace_fptrs() < 0) {
>>> + syslog(LOG_WARNING, "backtrace symbols not found, no fatal
>>> signal handlers will be installed");
>>> + }
>>> + else {
>> No new line before the "else" keyword.
>>> +
>>> + // prepare a filename for backtrace
>>> + time_string[0] = '\0';
>>> +
>>> + if (time(¤t_time) < 0) {
>>> + syslog(LOG_WARNING, "time failed: %s", strerror(errno));
>>> + }
>>> + else {
>> No new line before the "else" keyword.
>>> + if (strftime(time_string, sizeof(time_string),
>>> "%Y%m%d_%T", localtime(¤t_time)) == 0) {
>>> + syslog(LOG_WARNING, "strftime failed");
>>> + }
>>> + }
>>> +
>>> + snprintf(bt_filename, sizeof(bt_filename), PKGLOGDIR
>>> "/bt_%s_%d", time_string, getpid());
>>> +
>>> + memset(&action, 0, sizeof(action));
>>> + action.sa_sigaction = fatal_signal_handler;
>>> + sigfillset(&action.sa_mask);
>>> + action.sa_flags = SA_RESETHAND | SA_SIGINFO;
>>> +
>>> + for (i = 0; i < HANDLED_SIGNALS_MAX; ++i)
>>> + {
>> No new line before opening brace.
>>> + if (sigaction(handled_signals[i], &action, NULL) < 0)
>>> + {
>> No new line before opening brace.
>>> + syslog(LOG_WARNING, "sigaction %d failed: %s",
>>> handled_signals[i], strerror(errno));
>>> + }
>>> + }
>>> + }
>>> +}
>>
>
------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
