google / glog

C++ implementation of the Google logging module
http://google.github.io/glog/
BSD 3-Clause "New" or "Revised" License
7.01k stars 2.05k forks source link

Using call_once could lead to infinite loop in fatal signal handler ? #1080

Closed Haoqian-He closed 5 months ago

Haoqian-He commented 7 months ago

Hi glog experts, i wonder if using call_once (in this patch) which leading to infinite loop in such a scenario:

  1. we get SIGSEGV and into signal handler
  2. in signal handler trigger SIGABRT
  3. infintie loop...

In short, another fatal signal is triggered in the signal handler which may cause an infinite loop.

@sergiud #1026

sergiud commented 7 months ago

I'm not sure I fully understand the problem. Do you have an example that demonstrates the issue?

Haoqian-He commented 7 months ago

sorry, i write a demo only for test.

#include <cstdio>
#include <string.h>
#include <pthread.h>
#include <csignal>
#include <unistd.h>
#include <limits.h>
#include <mutex>

using namespace std;

void *signal_thread(void *arg) {
    int i = 0;
    while(1) {
        i++;
        printf("child 1 %d\n", i);
        if (i > 3) {
            raise(SIGSEGV);
        }
        sleep(5);
    }

}

static std::once_flag signaled;

void FailureSignalHandler(int sig) {
    int i = 0;
    while(i < INT_MAX) {
        i++;
    }

    raise(SIGABRT);
    struct sigaction sig_action;
    memset(&sig_action, 0, sizeof(sig_action));
    sigemptyset(&sig_action.sa_mask);
    sig_action.sa_handler = SIG_DFL;
    sigaction(sig, &sig_action, NULL);
    printf("x %d\n", sig);
    kill(getpid(), sig);
}

static void FatalSignalHandler(int sig, siginfo_t* info, void* data) {
    printf("sig %d\n", sig);
    std::call_once(signaled, &FailureSignalHandler, sig);
    printf("%d\n", sig);
}

int main(int argc, char *argv[]) {
    pthread_t tid;
    int rc;

    struct sigaction sig_action;
    memset(&sig_action, 0, sizeof(sig_action));
    sig_action.sa_flags = SA_SIGINFO;
    sig_action.sa_sigaction = FatalSignalHandler;
    sigemptyset(&sig_action.sa_mask);
    sigaction(SIGSEGV, &sig_action, nullptr);
    sigaction(SIGABRT, &sig_action, nullptr);

    rc = pthread_create(&tid, NULL, signal_thread, NULL);
    if (rc != 0) {
        exit(1);
    }

    pthread_join(tid, nullptr);
}

i compile this code with gcc 7 and run in a centos 7 (it occurs infinite loop

i'm not sure if we should consider this scenario in this patch

sergiud commented 7 months ago

I do not observe an infinite loop. In fact, it is pthread_join that causes a dead lock in your code. While FailureSignalHandler is called by std::call_once exactly a single time.

Haoqian-He commented 7 months ago

I do not observe an infinite loop. In fact, it is pthread_join that causes a dead lock in your code. While FailureSignalHandler is called by std::call_once exactly a single time.

yeah , FailureSignalHandler is called once, but single thread cannot quit. my output is:

child 1 1
child 1 2
child 1 3
child 1 4
sig 11
sig 6

It did not run printf("%d\n", sig); after call_once .

Here is gdb info, single thread doesn't quit so in main thread pthread_join always waitting.

(gdb) bt
#0  0x00007fc63aa451f5 in __pthread_once_slow () from /lib64/libpthread.so.0
#1  0x0000000000400aa5 in __gthread_once(int*, void (*)()) ()
#2  0x0000000000400dbe in void std::call_once<void (*)(int), int&>(std::once_flag&, void (*&&)(int), int&) ()
#3  0x0000000000400bed in FatalSignalHandler(int, siginfo_t*, void*) ()
#4  <signal handler called>
#5  0x00007fc63aa4e4fb in raise () from /lib64/libpthread.so.0
#6  0x0000000000400b26 in FailureSignalHandler(int) ()
#7  0x0000000000400e7d in void std::__invoke_impl<void, void (*)(int), int&>(std::__invoke_other, void (*&&)(int), int&) ()
#8  0x0000000000400e2f in std::__invoke_result<void (*)(int), int&>::type std::__invoke<void (*)(int), int&>(void (*&&)(int), int&) ()
#9  0x0000000000400d0c in void std::call_once<void (*)(int), int&>(std::once_flag&, void (*&&)(int), int&)::{lambda()#1}::operator()() const ()
#10 0x0000000000400d33 in void std::call_once<void (*)(int), int&>(std::once_flag&, void (*&&)(int), int&)::{lambda()#2}::operator()() const ()
#11 0x0000000000400d44 in void std::call_once<void (*)(int), int&>(std::once_flag&, void (*&&)(int), int&)::{lambda()#2}::_FUN() ()
#12 0x00007fc63aa4520b in __pthread_once_slow () from /lib64/libpthread.so.0
#13 0x0000000000400aa5 in __gthread_once(int*, void (*)()) ()
#14 0x0000000000400dbe in void std::call_once<void (*)(int), int&>(std::once_flag&, void (*&&)(int), int&) ()
#15 0x0000000000400bed in FatalSignalHandler(int, siginfo_t*, void*) ()
#16 <signal handler called>
#17 0x00007fc63aa4e4fb in raise () from /lib64/libpthread.so.0
#18 0x0000000000400ae9 in signal_thread(void*) ()
#19 0x00007fc63aa46ea5 in start_thread () from /lib64/libpthread.so.0
#20 0x00007fc639f50b0d in clone () from /lib64/libc.so.6

sorry i don't quite understand how the deadlock you mentioned occurs.

sergiud commented 7 months ago

It did not run printf("%d\n", sig); after call_once .

That's because your code installs a SIGABRT signal handler here

struct sigaction sig_action;
memset(&sig_action, 0, sizeof(sig_action));
sig_action.sa_flags = SA_SIGINFO;
sig_action.sa_sigaction = FatalSignalHandler;
sigemptyset(&sig_action.sa_mask);
sigaction(SIGSEGV, &sig_action, nullptr);
sigaction(SIGABRT, &sig_action, nullptr);

that infinitely raises SIGABRT without actually exiting the process.

However, I'm not sure how this is related to the std::call_once use in glog. Do you observe a similar behavior in glog as well? If this is the case, do you have a glog specific example?

Haoqian-He commented 7 months ago

Since i saw that the signal handlers of SIGSEGV and SIGABRT were also registered in glog, I thought that if another signal (in above demo is SIGABRT) is triggered during the execution of signal handler, an infinite loop may occur.

However, i really don't know if this scenario needs to be considered cause i haven't reproduced it yet, it's just a theoretical consideration.

sergiud commented 7 months ago

To avoid speculations a test case that actually uses glog would be helpful. Either way, I do not see any relation to std::call_once at the moment which is the reason for this whole discussion.

Haoqian-He commented 7 months ago

sure, it better to write an UT.

Anyway, the above demo register SIGSEGV & SIGABRT handler function like glog signal handler , both glog and demo use std::call_once in signal handler.

I will try to write an test case ASAP. Thx sergiud !

sergiud commented 5 months ago

Let me close this issue as there is nothing to address right now.