diff --git a/base/daemon/BaseDaemon.cpp b/base/daemon/BaseDaemon.cpp index 524fc5bbb49..b92a68f104e 100644 --- a/base/daemon/BaseDaemon.cpp +++ b/base/daemon/BaseDaemon.cpp @@ -104,7 +104,7 @@ static void writeSignalIDtoSignalPipe(int sig) errno = saved_errno; } -/** Signal handler for HUP / USR1 */ +/** Signal handler for HUP */ static void closeLogsSignalHandler(int sig, siginfo_t *, void *) { DENY_ALLOCATIONS_IN_SCOPE; @@ -161,7 +161,7 @@ __attribute__((__weak__)) void collectCrashLog( /** The thread that read info about signal or std::terminate from pipe. - * On HUP / USR1, close log files (for new files to be opened later). + * On HUP, close log files (for new files to be opened later). * On information about std::terminate, write it to log. * On other signals, write info to log. */ @@ -201,7 +201,7 @@ public: LOG_INFO(log, "Stop SignalListener thread"); break; } - else if (sig == SIGHUP || sig == SIGUSR1) + else if (sig == SIGHUP) { LOG_DEBUG(log, "Received signal to close logs."); BaseDaemon::instance().closeLogs(BaseDaemon::instance().logger()); @@ -832,7 +832,7 @@ void BaseDaemon::initializeTerminationAndSignalProcessing() /// SIGTSTP is added for debugging purposes. To output a stack trace of any running thread at anytime. addSignalHandler({SIGABRT, SIGSEGV, SIGILL, SIGBUS, SIGSYS, SIGFPE, SIGPIPE, SIGTSTP, SIGTRAP}, signalHandler, &handled_signals); - addSignalHandler({SIGHUP, SIGUSR1}, closeLogsSignalHandler, &handled_signals); + addSignalHandler({SIGHUP}, closeLogsSignalHandler, &handled_signals); addSignalHandler({SIGINT, SIGQUIT, SIGTERM}, terminateRequestedSignalHandler, &handled_signals); #if defined(SANITIZER) @@ -1006,7 +1006,7 @@ void BaseDaemon::setupWatchdog() /// Forward signals to the child process. addSignalHandler( - {SIGHUP, SIGUSR1, SIGINT, SIGQUIT, SIGTERM}, + {SIGHUP, SIGINT, SIGQUIT, SIGTERM}, [](int sig, siginfo_t *, void *) { /// Forward all signals except INT as it can be send by terminal to the process group when user press Ctrl+C, diff --git a/src/Common/QueryProfiler.cpp b/src/Common/QueryProfiler.cpp index 457443c7dc1..12410de6bf0 100644 --- a/src/Common/QueryProfiler.cpp +++ b/src/Common/QueryProfiler.cpp @@ -25,6 +25,12 @@ namespace { #if defined(OS_LINUX) thread_local size_t write_trace_iteration = 0; + /// Even after timer_delete() the signal can be delivered, + /// since it does not do anything with pending signals. + /// + /// And so to overcome this flag is exists, + /// to ignore delivered signals after timer_delete(). + thread_local bool signal_handler_disarmed = true; #endif void writeTraceInfo(TraceType trace_type, int /* sig */, siginfo_t * info, void * context) @@ -117,10 +123,8 @@ QueryProfilerBase::QueryProfilerBase(const UInt64 thread_id, const if (sigaddset(&sa.sa_mask, pause_signal)) throwFromErrno("Failed to add signal to mask for query profiler", ErrorCodes::CANNOT_MANIPULATE_SIGSET); - struct sigaction local_previous_handler; - if (sigaction(pause_signal, &sa, &local_previous_handler)) + if (sigaction(pause_signal, &sa, nullptr)) throwFromErrno("Failed to setup signal handler for query profiler", ErrorCodes::CANNOT_SET_SIGNAL_HANDLER); - previous_handler.emplace(local_previous_handler); try { @@ -160,6 +164,8 @@ QueryProfilerBase::QueryProfilerBase(const UInt64 thread_id, const struct itimerspec timer_spec = {.it_interval = interval, .it_value = offset}; if (timer_settime(*timer_id, 0, &timer_spec, nullptr)) throwFromErrno("Failed to set thread timer period", ErrorCodes::CANNOT_SET_TIMER_PERIOD); + + signal_handler_disarmed = false; } catch (...) { @@ -179,11 +185,14 @@ template void QueryProfilerBase::tryCleanup() { #if USE_UNWIND - if (timer_id.has_value() && timer_delete(*timer_id)) - LOG_ERROR(log, "Failed to delete query profiler timer {}", errnoToString(ErrorCodes::CANNOT_DELETE_TIMER)); + if (timer_id.has_value()) + { + if (timer_delete(*timer_id)) + LOG_ERROR(log, "Failed to delete query profiler timer {}", errnoToString(ErrorCodes::CANNOT_DELETE_TIMER)); + timer_id.reset(); + } - if (previous_handler.has_value() && sigaction(pause_signal, &*previous_handler, nullptr)) - LOG_ERROR(log, "Failed to restore signal handler after query profiler {}", errnoToString(ErrorCodes::CANNOT_SET_SIGNAL_HANDLER)); + signal_handler_disarmed = true; #endif } @@ -196,6 +205,9 @@ QueryProfilerReal::QueryProfilerReal(const UInt64 thread_id, const UInt32 period void QueryProfilerReal::signalHandler(int sig, siginfo_t * info, void * context) { + if (signal_handler_disarmed) + return; + DENY_ALLOCATIONS_IN_SCOPE; writeTraceInfo(TraceType::Real, sig, info, context); } @@ -206,6 +218,9 @@ QueryProfilerCPU::QueryProfilerCPU(const UInt64 thread_id, const UInt32 period) void QueryProfilerCPU::signalHandler(int sig, siginfo_t * info, void * context) { + if (signal_handler_disarmed) + return; + DENY_ALLOCATIONS_IN_SCOPE; writeTraceInfo(TraceType::CPU, sig, info, context); } diff --git a/src/Common/QueryProfiler.h b/src/Common/QueryProfiler.h index 5d011c6adfc..5cd64ecb76d 100644 --- a/src/Common/QueryProfiler.h +++ b/src/Common/QueryProfiler.h @@ -46,9 +46,6 @@ private: /// Pause signal to interrupt threads to get traces int pause_signal; - - /// Previous signal handler to restore after query profiler exits - std::optional previous_handler; }; /// Query profiler with timer based on real clock