From b75a472eea9aae934524c9d7f138adcfd8d3bdbd Mon Sep 17 00:00:00 2001 From: Zhe Zhang Date: Mon, 5 Oct 2015 23:24:28 -0500 Subject: [PATCH 1/7] Reimplement signal handler to avoid async-unsafe functions --- core/unix/inc/TUnixSystem.h | 23 ++++ core/unix/src/TUnixSystem.cxx | 231 +++++++++++++++++++++++++++++++++- 2 files changed, 251 insertions(+), 3 deletions(-) diff --git a/core/unix/inc/TUnixSystem.h b/core/unix/inc/TUnixSystem.h index 42abc6beb88ff..09660ea427cb4 100644 --- a/core/unix/inc/TUnixSystem.h +++ b/core/unix/inc/TUnixSystem.h @@ -31,11 +31,22 @@ #include "TTimer.h" #endif +#include +#include + +namespace std { + class thread; +} + +int global_stacktrace(void *); + typedef void (*SigHandler_t)(ESignals); class TUnixSystem : public TSystem { + friend int global_stacktrace(void *); + protected: const char *FindDynamicLibrary(TString &lib, Bool_t quiet = kFALSE); const char *GetLinkedLibraries(); @@ -71,6 +82,17 @@ class TUnixSystem : public TSystem { static int UnixRecv(int sock, void *buf, int len, int flag); static int UnixSend(int sock, const void *buf, int len, int flag); + // added helper static members for stacktrace + static const int pidStringLength_ = 255; + static char pidString_[pidStringLength_]; + static char * const pstackArgv_[]; + static char *const *getPstackArgv(); + static int parentToChild_[2]; + static int childToParent_[2]; + static std::unique_ptr helperThread_; + static void stacktraceHelperThread(); + void cachePidInfo(); + public: TUnixSystem(); virtual ~TUnixSystem(); @@ -121,6 +143,7 @@ class TUnixSystem : public TSystem { void Abort(int code = 0); int GetPid(); void StackTrace(); + static void stacktraceFromThread(); //---- Directories ------------------------------------------ int MakeDirectory(const char *name); diff --git a/core/unix/src/TUnixSystem.cxx b/core/unix/src/TUnixSystem.cxx index fc2df48f0bc8b..ffa98b5e1968e 100644 --- a/core/unix/src/TUnixSystem.cxx +++ b/core/unix/src/TUnixSystem.cxx @@ -568,12 +568,68 @@ TUnixSystem::~TUnixSystem() //////////////////////////////////////////////////////////////////////////////// /// Initialize Unix system interface. +extern "C" +{ + static int full_write(int fd, const char *text) + { + const char *buffer = text; + size_t count = strlen(text); + ssize_t written = 0; + while (count) + { + written = write(fd, buffer, count); + if (written == -1) + { + if (errno == EINTR) {continue;} + else {return -errno;} + } + count -= written; + buffer += written; + } + return 0; + } + + static int full_read(int fd, char *inbuf, size_t len) + { + char *buf = inbuf; + size_t count = len; + ssize_t complete = 0; + while (count) + { + complete = read(fd, buf, count); + if (complete == -1) + { + if (errno == EINTR) {continue;} + else {return -errno;} + } + count -= complete; + buf += complete; + } + return 0; + } + + static int full_cerr_write(const char *text) + { + return full_write(2, text); + } + +} + +static char pshellExec[8] = "/bin/sh"; +char TUnixSystem::pidString_[TUnixSystem::pidStringLength_] = {}; +static char pidNum[11] = {}; //--- The largest PID number could be converted to 10 characters. +char * const TUnixSystem::pstackArgv_[] = {pshellExec, TUnixSystem::pidString_, pidNum, nullptr}; +int TUnixSystem::parentToChild_[2] = {-1, -1}; +int TUnixSystem::childToParent_[2] = {-1, -1}; +std::unique_ptr TUnixSystem::helperThread_; Bool_t TUnixSystem::Init() { if (TSystem::Init()) return kTRUE; + cachePidInfo(); + fReadmask = new TFdSet; fWritemask = new TFdSet; fReadready = new TFdSet; @@ -2154,10 +2210,8 @@ void TUnixSystem::StackTrace() } gdbscript += " "; } - TString gdbmess = gEnv->GetValue("Root.StacktraceMessage", ""); gdbmess = gdbmess.Strip(); - std::cout.flush(); fflush(stdout); @@ -2286,7 +2340,6 @@ void TUnixSystem::StackTrace() fprintf(f, "%s\n", gdbmess.Data()); fclose(f); } - // use gdb to get stack trace #ifdef R__MACOSX gdbscript += GetExePath(); @@ -3524,6 +3577,7 @@ static void sighandler(int sig) void TUnixSystem::DispatchSignals(ESignals sig) { + const char* signalname = "unknown"; switch (sig) { case kSigAlarm: DispatchTimers(kFALSE); @@ -3532,8 +3586,14 @@ void TUnixSystem::DispatchSignals(ESignals sig) CheckChilds(); break; case kSigBus: + signalname = "bus error"; + break; case kSigSegmentationViolation: + signalname = "segmentation violation"; + break; case kSigIllegalInstruction: + signalname = "illegal instruction"; + break; case kSigFloatingException: Break("TUnixSystem::DispatchSignals", "%s", UnixSigname(sig)); StackTrace(); @@ -3558,6 +3618,20 @@ void TUnixSystem::DispatchSignals(ESignals sig) break; } + if ((sig == kSigIllegalInstruction) || (sig == kSigSegmentationViolation) || (sig == kSigBus)) + { + + full_cerr_write("\n\nA fatal system signal has occurred: "); + full_cerr_write(signalname); + full_cerr_write("\nThe following is the call stack containing the origin of the signal.\n" + "NOTE:The first few functions on the stack are artifacts of processing the signal and can be ignored\n\n"); + + TUnixSystem::stacktraceFromThread(); + + signal(sig, SIG_DFL); + raise(sig); + } + // check a-synchronous signals if (fSigcnt > 0 && fSignalHandler->GetSize() > 0) CheckSignals(kFALSE); @@ -5148,3 +5222,154 @@ int TUnixSystem::GetProcInfo(ProcInfo_t *info) const return 0; } + +static void stacktrace_fork(); + +void TUnixSystem::stacktraceHelperThread() +{ + int toParent = childToParent_[1]; + int fromParent = parentToChild_[0]; + char buf[2]; buf[1] = '\0'; + while(true) + { + int result = full_read(fromParent, buf, 1); + if (result < 0) + { + close(toParent); + full_cerr_write("\n\nTraceback helper thread failed to read from parent: "); + full_cerr_write(strerror(-result)); + full_cerr_write("\n"); + ::abort(); + } + if (buf[0] == '1') + { + stacktrace_fork(); + full_write(toParent, buf); + } + else if (buf[0] == '2') + { + close(toParent); + close(fromParent); + toParent = childToParent_[1]; + fromParent = parentToChild_[0]; + } + else if (buf[0] == '3') + { + break; + } + else + { + close(toParent); + full_cerr_write("\n\nTraceback helper thread got unknown command from parent: "); + full_cerr_write(buf); + full_cerr_write("\n"); + ::abort(); + } + } +} + +void TUnixSystem::stacktraceFromThread() +{ + int result = full_write(parentToChild_[1], "1"); + if (result < 0) + { + full_cerr_write("\n\nAttempt to request stacktrace failed: "); + full_cerr_write(strerror(-result)); + full_cerr_write("\n"); + return; + } + char buf[2]; buf[1] = '\0'; + if ((result = full_read(childToParent_[0], buf, 1)) < 0) + { + full_cerr_write("\n\nWaiting for stacktrace completion failed: "); + full_cerr_write(strerror(-result)); + full_cerr_write("\n"); + return; + } + +} + +void stacktrace_fork() +{ + char child_stack[4*1024]; + char *child_stack_ptr = child_stack + 4*1024; + int pid = +#ifdef __linux__ + clone(global_stacktrace, child_stack_ptr, CLONE_VM|CLONE_FS|SIGCHLD, nullptr); +#else + fork(); + if (child_stack_ptr) {} // Suppress 'unused variable' warning on non-Linux + if (pid == 0) {global_stacktrace(nullptr); ::abort();} +#endif + if (pid == -1) + { + full_cerr_write("(Attempt to perform stack dump failed.)\n"); + } + else + { + int status; + if (waitpid(pid, &status, 0) == -1) + { + full_cerr_write("(Failed to wait on stack dump output.)\n"); + } else {} + } +} + +int global_stacktrace(void * /*arg*/) +{ + char *const *argv = TUnixSystem::getPstackArgv(); + execv("/bin/sh", argv); + ::abort(); + return 1; +} + +char *const *TUnixSystem::getPstackArgv() { + return pstackArgv_; +} + +void TUnixSystem::cachePidInfo() +{ +#ifdef ROOTETCDIR + if(sprintf(pidString_, "%s/gdb-backtrace.sh", ROOTETCDIR) >= pidStringLength_) + { + full_cerr_write("Unable to pre-allocate executable information"); + exit(1); + } +#else + if(sprintf(pidString_, "%s/etc/gdb-backtrace.sh", Getenv("ROOTSYS")) >= pidStringLength_) + { + full_cerr_write("Unable to pre-allocate executable information"); + exit(1); + } +#endif + + if(sprintf(pidNum, "%d", getpid()) >= pidStringLength_) + { + full_cerr_write("Unable to pre-allocate process id information"); + exit(1); + } + + close(childToParent_[0]); + close(childToParent_[1]); + childToParent_[0] = -1; childToParent_[1] = -1; + close(parentToChild_[0]); + close(parentToChild_[1]); + parentToChild_[0] = -1; parentToChild_[1] = -1; + + if (-1 == pipe2(childToParent_, O_CLOEXEC)) + { + fprintf(stdout, "pipe childToParent failed\n"); + exit(1); + } + + if (-1 == pipe2(parentToChild_, O_CLOEXEC)) + { + close(childToParent_[0]); close(childToParent_[1]); + childToParent_[0] = -1; childToParent_[1] = -1; + fprintf(stdout, "pipe parentToChild failed\n"); + exit(1); + } + + helperThread_.reset(new std::thread(stacktraceHelperThread)); + helperThread_->detach(); +} From 3e349473832249e812fa049dfa28ed77a084dae4 Mon Sep 17 00:00:00 2001 From: Zhe Zhang Date: Sun, 18 Oct 2015 22:58:42 -0500 Subject: [PATCH 2/7] Add timeout option for FullRead function and fix the coding style --- core/unix/inc/TUnixSystem.h | 28 ++- core/unix/src/TUnixSystem.cxx | 352 ++++++++++++++++++---------------- 2 files changed, 201 insertions(+), 179 deletions(-) diff --git a/core/unix/inc/TUnixSystem.h b/core/unix/inc/TUnixSystem.h index 09660ea427cb4..dabaa6ffb6b26 100644 --- a/core/unix/inc/TUnixSystem.h +++ b/core/unix/inc/TUnixSystem.h @@ -34,18 +34,14 @@ #include #include -namespace std { - class thread; -} - -int global_stacktrace(void *); +int StackTraceExec(void *); typedef void (*SigHandler_t)(ESignals); class TUnixSystem : public TSystem { - friend int global_stacktrace(void *); + friend int StackTraceExec(void *); protected: const char *FindDynamicLibrary(TString &lib, Bool_t quiet = kFALSE); @@ -83,15 +79,15 @@ class TUnixSystem : public TSystem { static int UnixSend(int sock, const void *buf, int len, int flag); // added helper static members for stacktrace - static const int pidStringLength_ = 255; - static char pidString_[pidStringLength_]; - static char * const pstackArgv_[]; - static char *const *getPstackArgv(); - static int parentToChild_[2]; - static int childToParent_[2]; - static std::unique_ptr helperThread_; - static void stacktraceHelperThread(); - void cachePidInfo(); + static const int fPidStringLength = 255; + static char fPidString[fPidStringLength]; + static int fParentToChild[2]; + static int fChildToParent[2]; + static std::unique_ptr fHelperThread; + static char *const kStackArgv[]; + static char *const * GetStackArgv(); + static void StackTraceHelperThread(); + void CachePidInfo(); public: TUnixSystem(); @@ -143,7 +139,7 @@ class TUnixSystem : public TSystem { void Abort(int code = 0); int GetPid(); void StackTrace(); - static void stacktraceFromThread(); + static void StackTraceFromThread(); //---- Directories ------------------------------------------ int MakeDirectory(const char *name); diff --git a/core/unix/src/TUnixSystem.cxx b/core/unix/src/TUnixSystem.cxx index ffa98b5e1968e..4f6d88fa58928 100644 --- a/core/unix/src/TUnixSystem.cxx +++ b/core/unix/src/TUnixSystem.cxx @@ -40,10 +40,15 @@ #include #include +#ifdef __linux__ +#include +#endif + //#define G__OLDEXPAND #include #include +#include #include #if defined(R__SUN) || defined(R__AIX) || \ defined(R__LINUX) || defined(R__SOLARIS) || \ @@ -570,65 +575,99 @@ TUnixSystem::~TUnixSystem() /// Initialize Unix system interface. extern "C" { - static int full_write(int fd, const char *text) - { + static int FullWrite(int fd, const char *text) + { const char *buffer = text; size_t count = strlen(text); ssize_t written = 0; - while (count) - { - written = write(fd, buffer, count); - if (written == -1) - { - if (errno == EINTR) {continue;} - else {return -errno;} - } - count -= written; - buffer += written; + while (count) { + written = write(fd, buffer, count); + if (written == -1) { + if (errno == EINTR) { continue; } + else { return -errno; } + } + count -= written; + buffer += written; } return 0; - } + } - static int full_read(int fd, char *inbuf, size_t len) - { + static int FullRead(int fd, char *inbuf, size_t len, int timeout=-1) { char *buf = inbuf; size_t count = len; ssize_t complete = 0; - while (count) - { - complete = read(fd, buf, count); - if (complete == -1) - { - if (errno == EINTR) {continue;} - else {return -errno;} - } - count -= complete; - buf += complete; + std::chrono::time_point endTime = std::chrono::steady_clock::now() + std::chrono::seconds(timeout); + int flags; + if (timeout < 0) { + flags = O_NONBLOCK; // Prevents us from trying to set / restore flags later. + } else if ((-1 == (flags = fcntl(fd, F_GETFL)))) { + return -errno; + } else { } + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + if (-1 == fcntl(fd, F_SETFL, flags | O_NONBLOCK)) { + return -errno; + } + } + while (count) { + if (timeout >= 0) { + struct pollfd pollInfo{fd, POLLIN, 0}; + int msRemaining = std::chrono::duration_cast(endTime-std::chrono::steady_clock::now()).count(); + if (msRemaining > 0) { + if (poll(&pollInfo, 1, msRemaining) == 0) { + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + fcntl(fd, F_SETFL, flags); + } + return -ETIMEDOUT; + } + } else if (msRemaining < 0) { + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + fcntl(fd, F_SETFL, flags); + } + return -ETIMEDOUT; + } else { } + } + complete = read(fd, buf, count); + if (complete == -1) { + if (errno == EINTR) { continue; } + else if ((errno == EAGAIN) || (errno == EWOULDBLOCK)) { continue; } + else { + int origErrno = errno; + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + fcntl(fd, F_SETFL, flags); + } + return -origErrno; + } + } + count -= complete; + buf += complete; + } + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + fcntl(fd, F_SETFL, flags); } return 0; - } + } - static int full_cerr_write(const char *text) - { - return full_write(2, text); - } + static int FullErrWrite(const char *text) { + return FullWrite(2, text); + } } -static char pshellExec[8] = "/bin/sh"; -char TUnixSystem::pidString_[TUnixSystem::pidStringLength_] = {}; -static char pidNum[11] = {}; //--- The largest PID number could be converted to 10 characters. -char * const TUnixSystem::pstackArgv_[] = {pshellExec, TUnixSystem::pidString_, pidNum, nullptr}; -int TUnixSystem::parentToChild_[2] = {-1, -1}; -int TUnixSystem::childToParent_[2] = {-1, -1}; -std::unique_ptr TUnixSystem::helperThread_; +std::unique_ptr TUnixSystem::fHelperThread; + +static char shellExec[] = "/bin/sh"; +char TUnixSystem::fPidString[TUnixSystem::fPidStringLength] = {}; +static char pidNum[11] = {}; //--- The largest PID number could be converted to 10 characters. +char * const TUnixSystem::kStackArgv[] = {shellExec, TUnixSystem::fPidString, pidNum, nullptr}; +int TUnixSystem::fParentToChild[2] = {-1, -1}; +int TUnixSystem::fChildToParent[2] = {-1, -1}; Bool_t TUnixSystem::Init() { if (TSystem::Init()) return kTRUE; - cachePidInfo(); + CachePidInfo(); fReadmask = new TFdSet; fWritemask = new TFdSet; @@ -3621,12 +3660,12 @@ void TUnixSystem::DispatchSignals(ESignals sig) if ((sig == kSigIllegalInstruction) || (sig == kSigSegmentationViolation) || (sig == kSigBus)) { - full_cerr_write("\n\nA fatal system signal has occurred: "); - full_cerr_write(signalname); - full_cerr_write("\nThe following is the call stack containing the origin of the signal.\n" + FullErrWrite("\n\nA fatal system signal has occurred: "); + FullErrWrite(signalname); + FullErrWrite("\nThe following is the call stack containing the origin of the signal.\n" "NOTE:The first few functions on the stack are artifacts of processing the signal and can be ignored\n\n"); - TUnixSystem::stacktraceFromThread(); + TUnixSystem::StackTraceFromThread(); signal(sig, SIG_DFL); raise(sig); @@ -5223,153 +5262,140 @@ int TUnixSystem::GetProcInfo(ProcInfo_t *info) const return 0; } -static void stacktrace_fork(); +static void StackTraceFork(); + +void SetDefaultSignals() { + signal(SIGILL, SIG_DFL); + signal(SIGSEGV, SIG_DFL); + signal(SIGBUS, SIG_DFL); +} -void TUnixSystem::stacktraceHelperThread() +void TUnixSystem::StackTraceHelperThread() { - int toParent = childToParent_[1]; - int fromParent = parentToChild_[0]; - char buf[2]; buf[1] = '\0'; - while(true) - { - int result = full_read(fromParent, buf, 1); - if (result < 0) - { - close(toParent); - full_cerr_write("\n\nTraceback helper thread failed to read from parent: "); - full_cerr_write(strerror(-result)); - full_cerr_write("\n"); - ::abort(); - } - if (buf[0] == '1') - { - stacktrace_fork(); - full_write(toParent, buf); - } - else if (buf[0] == '2') - { + int toParent = fChildToParent[1]; + int fromParent = fParentToChild[0]; + char buf[2]; buf[1] = '\0'; + while(true) { + int result = FullRead(fromParent, buf, 1, 5*60); + if (result < 0) { + SetDefaultSignals(); + close(toParent); + FullErrWrite("\n\nTraceback helper thread failed to read from parent: "); + FullErrWrite(strerror(-result)); + FullErrWrite("\n"); + ::abort(); + } + if (buf[0] == '1') { + SetDefaultSignals(); + StackTraceFork(); + FullWrite(toParent, buf); + } else if (buf[0] == '2') { close(toParent); close(fromParent); - toParent = childToParent_[1]; - fromParent = parentToChild_[0]; - } - else if (buf[0] == '3') - { + toParent = fChildToParent[1]; + fromParent = fParentToChild[0]; + } else if (buf[0] == '3') { break; - } - else - { + } else { + SetDefaultSignals(); close(toParent); - full_cerr_write("\n\nTraceback helper thread got unknown command from parent: "); - full_cerr_write(buf); - full_cerr_write("\n"); + FullErrWrite("\n\nTraceback helper thread got unknown command from parent: "); + FullErrWrite(buf); + FullErrWrite("\n"); ::abort(); - } } + } } -void TUnixSystem::stacktraceFromThread() +void TUnixSystem::StackTraceFromThread() { - int result = full_write(parentToChild_[1], "1"); - if (result < 0) - { - full_cerr_write("\n\nAttempt to request stacktrace failed: "); - full_cerr_write(strerror(-result)); - full_cerr_write("\n"); - return; - } - char buf[2]; buf[1] = '\0'; - if ((result = full_read(childToParent_[0], buf, 1)) < 0) - { - full_cerr_write("\n\nWaiting for stacktrace completion failed: "); - full_cerr_write(strerror(-result)); - full_cerr_write("\n"); - return; - } - + int result = FullWrite(fParentToChild[1], "1"); + if (result < 0) { + FullErrWrite("\n\nAttempt to request stacktrace failed: "); + FullErrWrite(strerror(-result)); + FullErrWrite("\n"); + return; + } + char buf[2]; buf[1] = '\0'; + if ((result = FullRead(fChildToParent[0], buf, 1)) < 0) { + FullErrWrite("\n\nWaiting for stacktrace completion failed: "); + FullErrWrite(strerror(-result)); + FullErrWrite("\n"); + return; + } } -void stacktrace_fork() +void StackTraceFork() { - char child_stack[4*1024]; - char *child_stack_ptr = child_stack + 4*1024; - int pid = + char childStack[4*1024]; + char *childStackPtr = childStack + 4*1024; + int pid = #ifdef __linux__ - clone(global_stacktrace, child_stack_ptr, CLONE_VM|CLONE_FS|SIGCHLD, nullptr); + clone(StackTraceExec, childStackPtr, CLONE_VM|CLONE_FS|SIGCHLD, nullptr); #else - fork(); - if (child_stack_ptr) {} // Suppress 'unused variable' warning on non-Linux - if (pid == 0) {global_stacktrace(nullptr); ::abort();} + fork(); + if (childStackPtr) {} // Suppress 'unused variable' warning on non-Linux + if (pid == 0) { StackTraceExec(nullptr); ::abort(); } #endif - if (pid == -1) - { - full_cerr_write("(Attempt to perform stack dump failed.)\n"); - } - else - { - int status; - if (waitpid(pid, &status, 0) == -1) - { - full_cerr_write("(Failed to wait on stack dump output.)\n"); - } else {} - } + if (pid == -1) { + FullErrWrite("(Attempt to perform stack dump failed.)\n"); + } else { + int status; + if (waitpid(pid, &status, 0) == -1) { + FullErrWrite("(Failed to wait on stack dump output.)\n"); + } else {} + } } -int global_stacktrace(void * /*arg*/) +int StackTraceExec(void *) { - char *const *argv = TUnixSystem::getPstackArgv(); - execv("/bin/sh", argv); - ::abort(); - return 1; + char *const *argv = TUnixSystem::GetStackArgv(); +#ifdef __linux__ + syscall(SYS_execve, "/bin/sh", argv, __environ); +#else + execv("/bin/sh", argv); +#endif + ::abort(); + return 1; } -char *const *TUnixSystem::getPstackArgv() { - return pstackArgv_; +char *const *TUnixSystem::GetStackArgv() { + return kStackArgv; } -void TUnixSystem::cachePidInfo() +void TUnixSystem::CachePidInfo() { #ifdef ROOTETCDIR - if(sprintf(pidString_, "%s/gdb-backtrace.sh", ROOTETCDIR) >= pidStringLength_) - { - full_cerr_write("Unable to pre-allocate executable information"); - exit(1); - } + if(sprintf(fPidString, "%s/gdb-backtrace.sh", ROOTETCDIR) >= fPidStringLength) { + FullErrWrite("Unable to pre-allocate executable information"); + return; + } #else - if(sprintf(pidString_, "%s/etc/gdb-backtrace.sh", Getenv("ROOTSYS")) >= pidStringLength_) - { - full_cerr_write("Unable to pre-allocate executable information"); - exit(1); - } + if(sprintf(fPidString, "%s/etc/gdb-backtrace.sh", Getenv("ROOTSYS")) >= fPidStringLength) { + FullErrWrite("Unable to pre-allocate executable information"); + return; + } #endif - - if(sprintf(pidNum, "%d", getpid()) >= pidStringLength_) - { - full_cerr_write("Unable to pre-allocate process id information"); - exit(1); - } - - close(childToParent_[0]); - close(childToParent_[1]); - childToParent_[0] = -1; childToParent_[1] = -1; - close(parentToChild_[0]); - close(parentToChild_[1]); - parentToChild_[0] = -1; parentToChild_[1] = -1; - - if (-1 == pipe2(childToParent_, O_CLOEXEC)) - { - fprintf(stdout, "pipe childToParent failed\n"); - exit(1); - } - - if (-1 == pipe2(parentToChild_, O_CLOEXEC)) - { - close(childToParent_[0]); close(childToParent_[1]); - childToParent_[0] = -1; childToParent_[1] = -1; - fprintf(stdout, "pipe parentToChild failed\n"); - exit(1); - } - - helperThread_.reset(new std::thread(stacktraceHelperThread)); - helperThread_->detach(); + if(sprintf(pidNum, "%d", GetPid()) >= fPidStringLength) { + FullErrWrite("Unable to pre-allocate process id information"); + return; + } + close(fChildToParent[0]); + close(fChildToParent[1]); + fChildToParent[0] = -1; fChildToParent[1] = -1; + close(fParentToChild[0]); + close(fParentToChild[1]); + fParentToChild[0] = -1; fParentToChild[1] = -1; + if (-1 == pipe2(fChildToParent, O_CLOEXEC)) { + fprintf(stdout, "pipe fChildToParent failed\n"); + return; + } + if (-1 == pipe2(fParentToChild, O_CLOEXEC)){ + close(fChildToParent[0]); close(fChildToParent[1]); + fChildToParent[0] = -1; fChildToParent[1] = -1; + fprintf(stdout, "pipe parentToChild failed\n"); + return; + } + fHelperThread.reset(new std::thread(StackTraceHelperThread)); + fHelperThread->detach(); } From 44131ee668019c83b790d426cb286e6ebd2e4ab8 Mon Sep 17 00:00:00 2001 From: Zhe Zhang Date: Mon, 19 Oct 2015 01:12:07 -0500 Subject: [PATCH 3/7] Move CachePidInfo after the path of ROOTSYS initialized --- core/unix/src/TUnixSystem.cxx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/core/unix/src/TUnixSystem.cxx b/core/unix/src/TUnixSystem.cxx index 4f6d88fa58928..6a67af3936da0 100644 --- a/core/unix/src/TUnixSystem.cxx +++ b/core/unix/src/TUnixSystem.cxx @@ -667,8 +667,6 @@ Bool_t TUnixSystem::Init() if (TSystem::Init()) return kTRUE; - CachePidInfo(); - fReadmask = new TFdSet; fWritemask = new TFdSet; fReadready = new TFdSet; @@ -703,6 +701,8 @@ Bool_t TUnixSystem::Init() gRootDir = ROOTPREFIX; #endif + CachePidInfo(); + return kFALSE; } @@ -5371,21 +5371,23 @@ void TUnixSystem::CachePidInfo() return; } #else - if(sprintf(fPidString, "%s/etc/gdb-backtrace.sh", Getenv("ROOTSYS")) >= fPidStringLength) { + if(sprintf(fPidString, "%s/etc/gdb-backtrace.sh", gSystem->Getenv("ROOTSYS")) >= fPidStringLength) { FullErrWrite("Unable to pre-allocate executable information"); return; - } + } #endif if(sprintf(pidNum, "%d", GetPid()) >= fPidStringLength) { FullErrWrite("Unable to pre-allocate process id information"); return; } + close(fChildToParent[0]); close(fChildToParent[1]); fChildToParent[0] = -1; fChildToParent[1] = -1; close(fParentToChild[0]); close(fParentToChild[1]); fParentToChild[0] = -1; fParentToChild[1] = -1; + if (-1 == pipe2(fChildToParent, O_CLOEXEC)) { fprintf(stdout, "pipe fChildToParent failed\n"); return; @@ -5396,6 +5398,7 @@ void TUnixSystem::CachePidInfo() fprintf(stdout, "pipe parentToChild failed\n"); return; } + fHelperThread.reset(new std::thread(StackTraceHelperThread)); fHelperThread->detach(); } From c67e55ffb3e2ad79c0a06dc035295caeab0608c7 Mon Sep 17 00:00:00 2001 From: Zhe Zhang Date: Mon, 19 Oct 2015 14:51:13 -0500 Subject: [PATCH 4/7] Change function name to SignalSafe* --- core/unix/src/TUnixSystem.cxx | 204 +++++++++++++++++----------------- 1 file changed, 101 insertions(+), 103 deletions(-) diff --git a/core/unix/src/TUnixSystem.cxx b/core/unix/src/TUnixSystem.cxx index 6a67af3936da0..9012e2d1cdf83 100644 --- a/core/unix/src/TUnixSystem.cxx +++ b/core/unix/src/TUnixSystem.cxx @@ -391,6 +391,83 @@ class TFdSet { ULong_t *GetBits() { return (ULong_t *)fds_bits; } }; +//////////////////////////////////////////////////////////////////////////////// +/// Async-signal-safe Read/Write functions. +static int SignalSafeWrite(int fd, const char *text) { + const char *buffer = text; + size_t count = strlen(text); + ssize_t written = 0; + while (count) { + written = write(fd, buffer, count); + if (written == -1) { + if (errno == EINTR) { continue; } + else { return -errno; } + } + count -= written; + buffer += written; + } + return 0; +} + +static int SignalSafeRead(int fd, char *inbuf, size_t len, int timeout=-1) { + char *buf = inbuf; + size_t count = len; + ssize_t complete = 0; + std::chrono::time_point endTime = std::chrono::steady_clock::now() + std::chrono::seconds(timeout); + int flags; + if (timeout < 0) { + flags = O_NONBLOCK; // Prevents us from trying to set / restore flags later. + } else if ((-1 == (flags = fcntl(fd, F_GETFL)))) { + return -errno; + } else { } + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + if (-1 == fcntl(fd, F_SETFL, flags | O_NONBLOCK)) { + return -errno; + } + } + while (count) { + if (timeout >= 0) { + struct pollfd pollInfo{fd, POLLIN, 0}; + int msRemaining = std::chrono::duration_cast(endTime-std::chrono::steady_clock::now()).count(); + if (msRemaining > 0) { + if (poll(&pollInfo, 1, msRemaining) == 0) { + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + fcntl(fd, F_SETFL, flags); + } + return -ETIMEDOUT; + } + } else if (msRemaining < 0) { + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + fcntl(fd, F_SETFL, flags); + } + return -ETIMEDOUT; + } else { } + } + complete = read(fd, buf, count); + if (complete == -1) { + if (errno == EINTR) { continue; } + else if ((errno == EAGAIN) || (errno == EWOULDBLOCK)) { continue; } + else { + int origErrno = errno; + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + fcntl(fd, F_SETFL, flags); + } + return -origErrno; + } + } + count -= complete; + buf += complete; + } + if ((flags & O_NONBLOCK) != O_NONBLOCK) { + fcntl(fd, F_SETFL, flags); + } + return 0; +} + +static int SignalSafeErrWrite(const char *text) { + return SignalSafeWrite(2, text); +} + //////////////////////////////////////////////////////////////////////////////// /// Unix signal handler. @@ -573,85 +650,6 @@ TUnixSystem::~TUnixSystem() //////////////////////////////////////////////////////////////////////////////// /// Initialize Unix system interface. -extern "C" -{ - static int FullWrite(int fd, const char *text) - { - const char *buffer = text; - size_t count = strlen(text); - ssize_t written = 0; - while (count) { - written = write(fd, buffer, count); - if (written == -1) { - if (errno == EINTR) { continue; } - else { return -errno; } - } - count -= written; - buffer += written; - } - return 0; - } - - static int FullRead(int fd, char *inbuf, size_t len, int timeout=-1) { - char *buf = inbuf; - size_t count = len; - ssize_t complete = 0; - std::chrono::time_point endTime = std::chrono::steady_clock::now() + std::chrono::seconds(timeout); - int flags; - if (timeout < 0) { - flags = O_NONBLOCK; // Prevents us from trying to set / restore flags later. - } else if ((-1 == (flags = fcntl(fd, F_GETFL)))) { - return -errno; - } else { } - if ((flags & O_NONBLOCK) != O_NONBLOCK) { - if (-1 == fcntl(fd, F_SETFL, flags | O_NONBLOCK)) { - return -errno; - } - } - while (count) { - if (timeout >= 0) { - struct pollfd pollInfo{fd, POLLIN, 0}; - int msRemaining = std::chrono::duration_cast(endTime-std::chrono::steady_clock::now()).count(); - if (msRemaining > 0) { - if (poll(&pollInfo, 1, msRemaining) == 0) { - if ((flags & O_NONBLOCK) != O_NONBLOCK) { - fcntl(fd, F_SETFL, flags); - } - return -ETIMEDOUT; - } - } else if (msRemaining < 0) { - if ((flags & O_NONBLOCK) != O_NONBLOCK) { - fcntl(fd, F_SETFL, flags); - } - return -ETIMEDOUT; - } else { } - } - complete = read(fd, buf, count); - if (complete == -1) { - if (errno == EINTR) { continue; } - else if ((errno == EAGAIN) || (errno == EWOULDBLOCK)) { continue; } - else { - int origErrno = errno; - if ((flags & O_NONBLOCK) != O_NONBLOCK) { - fcntl(fd, F_SETFL, flags); - } - return -origErrno; - } - } - count -= complete; - buf += complete; - } - if ((flags & O_NONBLOCK) != O_NONBLOCK) { - fcntl(fd, F_SETFL, flags); - } - return 0; - } - - static int FullErrWrite(const char *text) { - return FullWrite(2, text); - } - -} std::unique_ptr TUnixSystem::fHelperThread; @@ -3660,9 +3658,9 @@ void TUnixSystem::DispatchSignals(ESignals sig) if ((sig == kSigIllegalInstruction) || (sig == kSigSegmentationViolation) || (sig == kSigBus)) { - FullErrWrite("\n\nA fatal system signal has occurred: "); - FullErrWrite(signalname); - FullErrWrite("\nThe following is the call stack containing the origin of the signal.\n" + SignalSafeErrWrite("\n\nA fatal system signal has occurred: "); + SignalSafeErrWrite(signalname); + SignalSafeErrWrite("\nThe following is the call stack containing the origin of the signal.\n" "NOTE:The first few functions on the stack are artifacts of processing the signal and can be ignored\n\n"); TUnixSystem::StackTraceFromThread(); @@ -5276,19 +5274,19 @@ void TUnixSystem::StackTraceHelperThread() int fromParent = fParentToChild[0]; char buf[2]; buf[1] = '\0'; while(true) { - int result = FullRead(fromParent, buf, 1, 5*60); + int result = SignalSafeRead(fromParent, buf, 1, 5*60); if (result < 0) { SetDefaultSignals(); close(toParent); - FullErrWrite("\n\nTraceback helper thread failed to read from parent: "); - FullErrWrite(strerror(-result)); - FullErrWrite("\n"); + SignalSafeErrWrite("\n\nTraceback helper thread failed to read from parent: "); + SignalSafeErrWrite(strerror(-result)); + SignalSafeErrWrite("\n"); ::abort(); } if (buf[0] == '1') { SetDefaultSignals(); StackTraceFork(); - FullWrite(toParent, buf); + SignalSafeWrite(toParent, buf); } else if (buf[0] == '2') { close(toParent); close(fromParent); @@ -5299,9 +5297,9 @@ void TUnixSystem::StackTraceHelperThread() } else { SetDefaultSignals(); close(toParent); - FullErrWrite("\n\nTraceback helper thread got unknown command from parent: "); - FullErrWrite(buf); - FullErrWrite("\n"); + SignalSafeErrWrite("\n\nTraceback helper thread got unknown command from parent: "); + SignalSafeErrWrite(buf); + SignalSafeErrWrite("\n"); ::abort(); } } @@ -5309,18 +5307,18 @@ void TUnixSystem::StackTraceHelperThread() void TUnixSystem::StackTraceFromThread() { - int result = FullWrite(fParentToChild[1], "1"); + int result = SignalSafeWrite(fParentToChild[1], "1"); if (result < 0) { - FullErrWrite("\n\nAttempt to request stacktrace failed: "); - FullErrWrite(strerror(-result)); - FullErrWrite("\n"); + SignalSafeErrWrite("\n\nAttempt to request stacktrace failed: "); + SignalSafeErrWrite(strerror(-result)); + SignalSafeErrWrite("\n"); return; } char buf[2]; buf[1] = '\0'; - if ((result = FullRead(fChildToParent[0], buf, 1)) < 0) { - FullErrWrite("\n\nWaiting for stacktrace completion failed: "); - FullErrWrite(strerror(-result)); - FullErrWrite("\n"); + if ((result = SignalSafeRead(fChildToParent[0], buf, 1)) < 0) { + SignalSafeErrWrite("\n\nWaiting for stacktrace completion failed: "); + SignalSafeErrWrite(strerror(-result)); + SignalSafeErrWrite("\n"); return; } } @@ -5338,11 +5336,11 @@ void StackTraceFork() if (pid == 0) { StackTraceExec(nullptr); ::abort(); } #endif if (pid == -1) { - FullErrWrite("(Attempt to perform stack dump failed.)\n"); + SignalSafeErrWrite("(Attempt to perform stack dump failed.)\n"); } else { int status; if (waitpid(pid, &status, 0) == -1) { - FullErrWrite("(Failed to wait on stack dump output.)\n"); + SignalSafeErrWrite("(Failed to wait on stack dump output.)\n"); } else {} } } @@ -5367,17 +5365,17 @@ void TUnixSystem::CachePidInfo() { #ifdef ROOTETCDIR if(sprintf(fPidString, "%s/gdb-backtrace.sh", ROOTETCDIR) >= fPidStringLength) { - FullErrWrite("Unable to pre-allocate executable information"); + SignalSafeErrWrite("Unable to pre-allocate executable information"); return; } #else if(sprintf(fPidString, "%s/etc/gdb-backtrace.sh", gSystem->Getenv("ROOTSYS")) >= fPidStringLength) { - FullErrWrite("Unable to pre-allocate executable information"); + SignalSafeErrWrite("Unable to pre-allocate executable information"); return; } #endif if(sprintf(pidNum, "%d", GetPid()) >= fPidStringLength) { - FullErrWrite("Unable to pre-allocate process id information"); + SignalSafeErrWrite("Unable to pre-allocate process id information"); return; } From 0e6664c3ff80ca03a6c79445b8c6c5fe90a2812d Mon Sep 17 00:00:00 2001 From: Zhe Zhang Date: Mon, 19 Oct 2015 22:01:46 -0500 Subject: [PATCH 5/7] Add fShellExec and fPidNum into class TUnixSystem --- core/unix/inc/TUnixSystem.h | 2 ++ core/unix/src/TUnixSystem.cxx | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/core/unix/inc/TUnixSystem.h b/core/unix/inc/TUnixSystem.h index dabaa6ffb6b26..cb11aa7c4cac9 100644 --- a/core/unix/inc/TUnixSystem.h +++ b/core/unix/inc/TUnixSystem.h @@ -80,7 +80,9 @@ class TUnixSystem : public TSystem { // added helper static members for stacktrace static const int fPidStringLength = 255; + static char fShellExec[]; static char fPidString[fPidStringLength]; + static char fPidNum[11]; static int fParentToChild[2]; static int fChildToParent[2]; static std::unique_ptr fHelperThread; diff --git a/core/unix/src/TUnixSystem.cxx b/core/unix/src/TUnixSystem.cxx index 9012e2d1cdf83..870d7590b18a1 100644 --- a/core/unix/src/TUnixSystem.cxx +++ b/core/unix/src/TUnixSystem.cxx @@ -653,10 +653,10 @@ TUnixSystem::~TUnixSystem() std::unique_ptr TUnixSystem::fHelperThread; -static char shellExec[] = "/bin/sh"; +char TUnixSystem::fShellExec[] = "/bin/sh"; char TUnixSystem::fPidString[TUnixSystem::fPidStringLength] = {}; -static char pidNum[11] = {}; //--- The largest PID number could be converted to 10 characters. -char * const TUnixSystem::kStackArgv[] = {shellExec, TUnixSystem::fPidString, pidNum, nullptr}; +char TUnixSystem::fPidNum[11] = {}; //--- The largest PID number could be converted to 10 characters. +char * const TUnixSystem::kStackArgv[] = {TUnixSystem::fShellExec, TUnixSystem::fPidString, TUnixSystem::fPidNum, nullptr}; int TUnixSystem::fParentToChild[2] = {-1, -1}; int TUnixSystem::fChildToParent[2] = {-1, -1}; @@ -5374,7 +5374,7 @@ void TUnixSystem::CachePidInfo() return; } #endif - if(sprintf(pidNum, "%d", GetPid()) >= fPidStringLength) { + if(sprintf(fPidNum, "%d", GetPid()) >= fPidStringLength) { SignalSafeErrWrite("Unable to pre-allocate process id information"); return; } From 14dfb6a899c8bbb397cc51b8e235f07682550261 Mon Sep 17 00:00:00 2001 From: Zhe Zhang Date: Sun, 1 Nov 2015 23:23:52 -0600 Subject: [PATCH 6/7] Created private struct fStackTraceHelper for class TUnixSystem and moved initializations of shellExec and pidString into TUnixSystem::Init() --- core/unix/inc/TUnixSystem.h | 19 +++++--- core/unix/src/TUnixSystem.cxx | 83 +++++++++++++++++++---------------- 2 files changed, 56 insertions(+), 46 deletions(-) diff --git a/core/unix/inc/TUnixSystem.h b/core/unix/inc/TUnixSystem.h index cb11aa7c4cac9..8847ccff3e946 100644 --- a/core/unix/inc/TUnixSystem.h +++ b/core/unix/inc/TUnixSystem.h @@ -38,11 +38,23 @@ int StackTraceExec(void *); typedef void (*SigHandler_t)(ESignals); +struct TStackTraceHelper { + static const int stringLength = 255; + char shellExec[stringLength]; + char pidString[stringLength]; + char pidNum[stringLength]; + int parentToChild[2]; + int childToParent[2]; + std::unique_ptr helperThread; +}; class TUnixSystem : public TSystem { friend int StackTraceExec(void *); +private: + static struct TStackTraceHelper fStackTraceHelper; + protected: const char *FindDynamicLibrary(TString &lib, Bool_t quiet = kFALSE); const char *GetLinkedLibraries(); @@ -79,13 +91,6 @@ class TUnixSystem : public TSystem { static int UnixSend(int sock, const void *buf, int len, int flag); // added helper static members for stacktrace - static const int fPidStringLength = 255; - static char fShellExec[]; - static char fPidString[fPidStringLength]; - static char fPidNum[11]; - static int fParentToChild[2]; - static int fChildToParent[2]; - static std::unique_ptr fHelperThread; static char *const kStackArgv[]; static char *const * GetStackArgv(); static void StackTraceHelperThread(); diff --git a/core/unix/src/TUnixSystem.cxx b/core/unix/src/TUnixSystem.cxx index 870d7590b18a1..45376aad1d26f 100644 --- a/core/unix/src/TUnixSystem.cxx +++ b/core/unix/src/TUnixSystem.cxx @@ -651,14 +651,8 @@ TUnixSystem::~TUnixSystem() //////////////////////////////////////////////////////////////////////////////// /// Initialize Unix system interface. -std::unique_ptr TUnixSystem::fHelperThread; - -char TUnixSystem::fShellExec[] = "/bin/sh"; -char TUnixSystem::fPidString[TUnixSystem::fPidStringLength] = {}; -char TUnixSystem::fPidNum[11] = {}; //--- The largest PID number could be converted to 10 characters. -char * const TUnixSystem::kStackArgv[] = {TUnixSystem::fShellExec, TUnixSystem::fPidString, TUnixSystem::fPidNum, nullptr}; -int TUnixSystem::fParentToChild[2] = {-1, -1}; -int TUnixSystem::fChildToParent[2] = {-1, -1}; +struct TStackTraceHelper TUnixSystem::fStackTraceHelper; +char * const TUnixSystem::kStackArgv[] = {TUnixSystem::fStackTraceHelper.shellExec, TUnixSystem::fStackTraceHelper.pidString, TUnixSystem::fStackTraceHelper.pidNum, nullptr}; Bool_t TUnixSystem::Init() { @@ -699,6 +693,28 @@ Bool_t TUnixSystem::Init() gRootDir = ROOTPREFIX; #endif + if(snprintf(fStackTraceHelper.shellExec, fStackTraceHelper.stringLength-1, "/bin/sh") >= fStackTraceHelper.stringLength) { + SignalSafeErrWrite("Unable to pre-allocate shell command path"); + return kFALSE; + } + +#ifdef ROOTETCDIR + if(snprintf(fStackTraceHelper.pidString, fStackTraceHelper.stringLength-1, "%s/gdb-backtrace.sh", ROOTETCDIR) >= fStackTraceHelper.stringLength) { + SignalSafeErrWrite("Unable to pre-allocate executable information"); + return kFALSE; + } +#else + if(snprintf(fStackTraceHelper.pidString, fStackTraceHelper.stringLength-1, "%s/etc/gdb-backtrace.sh", gSystem->Getenv("ROOTSYS")) >= fStackTraceHelper.stringLength) { + SignalSafeErrWrite("Unable to pre-allocate executable information"); + return kFALSE; + } +#endif + + fStackTraceHelper.parentToChild[0] = -1; + fStackTraceHelper.parentToChild[1] = -1; + fStackTraceHelper.childToParent[0] = -1; + fStackTraceHelper.childToParent[1] = -1; + CachePidInfo(); return kFALSE; @@ -5270,8 +5286,8 @@ void SetDefaultSignals() { void TUnixSystem::StackTraceHelperThread() { - int toParent = fChildToParent[1]; - int fromParent = fParentToChild[0]; + int toParent = fStackTraceHelper.childToParent[1]; + int fromParent = fStackTraceHelper.parentToChild[0]; char buf[2]; buf[1] = '\0'; while(true) { int result = SignalSafeRead(fromParent, buf, 1, 5*60); @@ -5290,8 +5306,8 @@ void TUnixSystem::StackTraceHelperThread() } else if (buf[0] == '2') { close(toParent); close(fromParent); - toParent = fChildToParent[1]; - fromParent = fParentToChild[0]; + toParent = fStackTraceHelper.childToParent[1]; + fromParent = fStackTraceHelper.parentToChild[0]; } else if (buf[0] == '3') { break; } else { @@ -5307,7 +5323,7 @@ void TUnixSystem::StackTraceHelperThread() void TUnixSystem::StackTraceFromThread() { - int result = SignalSafeWrite(fParentToChild[1], "1"); + int result = SignalSafeWrite(fStackTraceHelper.parentToChild[1], "1"); if (result < 0) { SignalSafeErrWrite("\n\nAttempt to request stacktrace failed: "); SignalSafeErrWrite(strerror(-result)); @@ -5315,7 +5331,7 @@ void TUnixSystem::StackTraceFromThread() return; } char buf[2]; buf[1] = '\0'; - if ((result = SignalSafeRead(fChildToParent[0], buf, 1)) < 0) { + if ((result = SignalSafeRead(fStackTraceHelper.childToParent[0], buf, 1)) < 0) { SignalSafeErrWrite("\n\nWaiting for stacktrace completion failed: "); SignalSafeErrWrite(strerror(-result)); SignalSafeErrWrite("\n"); @@ -5363,40 +5379,29 @@ char *const *TUnixSystem::GetStackArgv() { void TUnixSystem::CachePidInfo() { -#ifdef ROOTETCDIR - if(sprintf(fPidString, "%s/gdb-backtrace.sh", ROOTETCDIR) >= fPidStringLength) { - SignalSafeErrWrite("Unable to pre-allocate executable information"); - return; - } -#else - if(sprintf(fPidString, "%s/etc/gdb-backtrace.sh", gSystem->Getenv("ROOTSYS")) >= fPidStringLength) { - SignalSafeErrWrite("Unable to pre-allocate executable information"); - return; - } -#endif - if(sprintf(fPidNum, "%d", GetPid()) >= fPidStringLength) { + if(snprintf(fStackTraceHelper.pidNum, fStackTraceHelper.stringLength-1, "%d", GetPid()) >= fStackTraceHelper.stringLength) { SignalSafeErrWrite("Unable to pre-allocate process id information"); return; } - close(fChildToParent[0]); - close(fChildToParent[1]); - fChildToParent[0] = -1; fChildToParent[1] = -1; - close(fParentToChild[0]); - close(fParentToChild[1]); - fParentToChild[0] = -1; fParentToChild[1] = -1; + close(fStackTraceHelper.childToParent[0]); + close(fStackTraceHelper.childToParent[1]); + fStackTraceHelper.childToParent[0] = -1; fStackTraceHelper.childToParent[1] = -1; + close(fStackTraceHelper.parentToChild[0]); + close(fStackTraceHelper.parentToChild[1]); + fStackTraceHelper.parentToChild[0] = -1; fStackTraceHelper.parentToChild[1] = -1; - if (-1 == pipe2(fChildToParent, O_CLOEXEC)) { - fprintf(stdout, "pipe fChildToParent failed\n"); + if (-1 == pipe2(fStackTraceHelper.childToParent, O_CLOEXEC)) { + fprintf(stdout, "pipe fStackTraceHelper.childToParent failed\n"); return; } - if (-1 == pipe2(fParentToChild, O_CLOEXEC)){ - close(fChildToParent[0]); close(fChildToParent[1]); - fChildToParent[0] = -1; fChildToParent[1] = -1; + if (-1 == pipe2(fStackTraceHelper.parentToChild, O_CLOEXEC)){ + close(fStackTraceHelper.childToParent[0]); close(fStackTraceHelper.childToParent[1]); + fStackTraceHelper.childToParent[0] = -1; fStackTraceHelper.childToParent[1] = -1; fprintf(stdout, "pipe parentToChild failed\n"); return; } - fHelperThread.reset(new std::thread(StackTraceHelperThread)); - fHelperThread->detach(); + fStackTraceHelper.helperThread.reset(new std::thread(StackTraceHelperThread)); + fStackTraceHelper.helperThread->detach(); } From 61ed59be2b7eeae8aba9a2bfe50e6bbd5f482ea8 Mon Sep 17 00:00:00 2001 From: Zhe Zhang Date: Tue, 5 Jan 2016 19:51:46 -0600 Subject: [PATCH 7/7] Change TStackTraceHelper as StackTraceHelper_t and move StackTraceHelper_t into TUnixSystem class --- core/unix/inc/TUnixSystem.h | 29 +++++++-------- core/unix/src/TUnixSystem.cxx | 67 ++++++++++++++++++----------------- 2 files changed, 50 insertions(+), 46 deletions(-) diff --git a/core/unix/inc/TUnixSystem.h b/core/unix/inc/TUnixSystem.h index 8847ccff3e946..d5f49c97100ff 100644 --- a/core/unix/inc/TUnixSystem.h +++ b/core/unix/inc/TUnixSystem.h @@ -38,22 +38,23 @@ int StackTraceExec(void *); typedef void (*SigHandler_t)(ESignals); -struct TStackTraceHelper { - static const int stringLength = 255; - char shellExec[stringLength]; - char pidString[stringLength]; - char pidNum[stringLength]; - int parentToChild[2]; - int childToParent[2]; - std::unique_ptr helperThread; -}; class TUnixSystem : public TSystem { friend int StackTraceExec(void *); private: - static struct TStackTraceHelper fStackTraceHelper; + struct StackTraceHelper_t { + static const int fStringLength = 255; + char fShellExec[fStringLength]; + char fPidString[fStringLength]; + char fPidNum[fStringLength]; + int fParentToChild[2]; + int fChildToParent[2]; + std::unique_ptr fHelperThread; + }; + + static StackTraceHelper_t fStackTraceHelper; protected: const char *FindDynamicLibrary(TString &lib, Bool_t quiet = kFALSE); @@ -91,10 +92,10 @@ class TUnixSystem : public TSystem { static int UnixSend(int sock, const void *buf, int len, int flag); // added helper static members for stacktrace - static char *const kStackArgv[]; - static char *const * GetStackArgv(); - static void StackTraceHelperThread(); - void CachePidInfo(); + static char *const kStackArgv[]; + static char *const *GetStackArgv(); + static void StackTraceHelperThread(); + void CachePidInfo(); public: TUnixSystem(); diff --git a/core/unix/src/TUnixSystem.cxx b/core/unix/src/TUnixSystem.cxx index 45376aad1d26f..863f286166dbf 100644 --- a/core/unix/src/TUnixSystem.cxx +++ b/core/unix/src/TUnixSystem.cxx @@ -392,7 +392,7 @@ class TFdSet { }; //////////////////////////////////////////////////////////////////////////////// -/// Async-signal-safe Read/Write functions. +/// Async-signal-safe Write functions. static int SignalSafeWrite(int fd, const char *text) { const char *buffer = text; size_t count = strlen(text); @@ -409,6 +409,8 @@ static int SignalSafeWrite(int fd, const char *text) { return 0; } +//////////////////////////////////////////////////////////////////////////////// +/// Async-signal-safe Read functions. static int SignalSafeRead(int fd, char *inbuf, size_t len, int timeout=-1) { char *buf = inbuf; size_t count = len; @@ -651,8 +653,8 @@ TUnixSystem::~TUnixSystem() //////////////////////////////////////////////////////////////////////////////// /// Initialize Unix system interface. -struct TStackTraceHelper TUnixSystem::fStackTraceHelper; -char * const TUnixSystem::kStackArgv[] = {TUnixSystem::fStackTraceHelper.shellExec, TUnixSystem::fStackTraceHelper.pidString, TUnixSystem::fStackTraceHelper.pidNum, nullptr}; +TUnixSystem::StackTraceHelper_t TUnixSystem::fStackTraceHelper; +char * const TUnixSystem::kStackArgv[] = {TUnixSystem::fStackTraceHelper.fShellExec, TUnixSystem::fStackTraceHelper.fPidString, TUnixSystem::fStackTraceHelper.fPidNum, nullptr}; Bool_t TUnixSystem::Init() { @@ -693,27 +695,27 @@ Bool_t TUnixSystem::Init() gRootDir = ROOTPREFIX; #endif - if(snprintf(fStackTraceHelper.shellExec, fStackTraceHelper.stringLength-1, "/bin/sh") >= fStackTraceHelper.stringLength) { + if(snprintf(fStackTraceHelper.fShellExec, fStackTraceHelper.fStringLength-1, "/bin/sh") >= fStackTraceHelper.fStringLength) { SignalSafeErrWrite("Unable to pre-allocate shell command path"); return kFALSE; } #ifdef ROOTETCDIR - if(snprintf(fStackTraceHelper.pidString, fStackTraceHelper.stringLength-1, "%s/gdb-backtrace.sh", ROOTETCDIR) >= fStackTraceHelper.stringLength) { + if(snprintf(fStackTraceHelper.fPidString, fStackTraceHelper.fStringLength-1, "%s/gdb-backtrace.sh", ROOTETCDIR) >= fStackTraceHelper.fStringLength) { SignalSafeErrWrite("Unable to pre-allocate executable information"); return kFALSE; } #else - if(snprintf(fStackTraceHelper.pidString, fStackTraceHelper.stringLength-1, "%s/etc/gdb-backtrace.sh", gSystem->Getenv("ROOTSYS")) >= fStackTraceHelper.stringLength) { + if(snprintf(fStackTraceHelper.fPidString, fStackTraceHelper.fStringLength-1, "%s/etc/gdb-backtrace.sh", gSystem->Getenv("ROOTSYS")) >= fStackTraceHelper.fStringLength) { SignalSafeErrWrite("Unable to pre-allocate executable information"); return kFALSE; } #endif - fStackTraceHelper.parentToChild[0] = -1; - fStackTraceHelper.parentToChild[1] = -1; - fStackTraceHelper.childToParent[0] = -1; - fStackTraceHelper.childToParent[1] = -1; + fStackTraceHelper.fParentToChild[0] = -1; + fStackTraceHelper.fParentToChild[1] = -1; + fStackTraceHelper.fChildToParent[0] = -1; + fStackTraceHelper.fChildToParent[1] = -1; CachePidInfo(); @@ -5286,8 +5288,8 @@ void SetDefaultSignals() { void TUnixSystem::StackTraceHelperThread() { - int toParent = fStackTraceHelper.childToParent[1]; - int fromParent = fStackTraceHelper.parentToChild[0]; + int toParent = fStackTraceHelper.fChildToParent[1]; + int fromParent = fStackTraceHelper.fParentToChild[0]; char buf[2]; buf[1] = '\0'; while(true) { int result = SignalSafeRead(fromParent, buf, 1, 5*60); @@ -5306,8 +5308,8 @@ void TUnixSystem::StackTraceHelperThread() } else if (buf[0] == '2') { close(toParent); close(fromParent); - toParent = fStackTraceHelper.childToParent[1]; - fromParent = fStackTraceHelper.parentToChild[0]; + toParent = fStackTraceHelper.fChildToParent[1]; + fromParent = fStackTraceHelper.fParentToChild[0]; } else if (buf[0] == '3') { break; } else { @@ -5323,7 +5325,7 @@ void TUnixSystem::StackTraceHelperThread() void TUnixSystem::StackTraceFromThread() { - int result = SignalSafeWrite(fStackTraceHelper.parentToChild[1], "1"); + int result = SignalSafeWrite(fStackTraceHelper.fParentToChild[1], "1"); if (result < 0) { SignalSafeErrWrite("\n\nAttempt to request stacktrace failed: "); SignalSafeErrWrite(strerror(-result)); @@ -5331,7 +5333,7 @@ void TUnixSystem::StackTraceFromThread() return; } char buf[2]; buf[1] = '\0'; - if ((result = SignalSafeRead(fStackTraceHelper.childToParent[0], buf, 1)) < 0) { + if ((result = SignalSafeRead(fStackTraceHelper.fChildToParent[0], buf, 1)) < 0) { SignalSafeErrWrite("\n\nWaiting for stacktrace completion failed: "); SignalSafeErrWrite(strerror(-result)); SignalSafeErrWrite("\n"); @@ -5341,8 +5343,9 @@ void TUnixSystem::StackTraceFromThread() void StackTraceFork() { - char childStack[4*1024]; - char *childStackPtr = childStack + 4*1024; + static const int stackSize = 4*1024; + char childStack[stackSize]; + char *childStackPtr = childStack + stackSize; int pid = #ifdef __linux__ clone(StackTraceExec, childStackPtr, CLONE_VM|CLONE_FS|SIGCHLD, nullptr); @@ -5379,29 +5382,29 @@ char *const *TUnixSystem::GetStackArgv() { void TUnixSystem::CachePidInfo() { - if(snprintf(fStackTraceHelper.pidNum, fStackTraceHelper.stringLength-1, "%d", GetPid()) >= fStackTraceHelper.stringLength) { + if(snprintf(fStackTraceHelper.fPidNum, fStackTraceHelper.fStringLength-1, "%d", GetPid()) >= fStackTraceHelper.fStringLength) { SignalSafeErrWrite("Unable to pre-allocate process id information"); return; } - close(fStackTraceHelper.childToParent[0]); - close(fStackTraceHelper.childToParent[1]); - fStackTraceHelper.childToParent[0] = -1; fStackTraceHelper.childToParent[1] = -1; - close(fStackTraceHelper.parentToChild[0]); - close(fStackTraceHelper.parentToChild[1]); - fStackTraceHelper.parentToChild[0] = -1; fStackTraceHelper.parentToChild[1] = -1; + close(fStackTraceHelper.fChildToParent[0]); + close(fStackTraceHelper.fChildToParent[1]); + fStackTraceHelper.fChildToParent[0] = -1; fStackTraceHelper.fChildToParent[1] = -1; + close(fStackTraceHelper.fParentToChild[0]); + close(fStackTraceHelper.fParentToChild[1]); + fStackTraceHelper.fParentToChild[0] = -1; fStackTraceHelper.fParentToChild[1] = -1; - if (-1 == pipe2(fStackTraceHelper.childToParent, O_CLOEXEC)) { - fprintf(stdout, "pipe fStackTraceHelper.childToParent failed\n"); + if (-1 == pipe2(fStackTraceHelper.fChildToParent, O_CLOEXEC)) { + fprintf(stdout, "pipe fStackTraceHelper.fChildToParent failed\n"); return; } - if (-1 == pipe2(fStackTraceHelper.parentToChild, O_CLOEXEC)){ - close(fStackTraceHelper.childToParent[0]); close(fStackTraceHelper.childToParent[1]); - fStackTraceHelper.childToParent[0] = -1; fStackTraceHelper.childToParent[1] = -1; + if (-1 == pipe2(fStackTraceHelper.fParentToChild, O_CLOEXEC)){ + close(fStackTraceHelper.fChildToParent[0]); close(fStackTraceHelper.fChildToParent[1]); + fStackTraceHelper.fChildToParent[0] = -1; fStackTraceHelper.fChildToParent[1] = -1; fprintf(stdout, "pipe parentToChild failed\n"); return; } - fStackTraceHelper.helperThread.reset(new std::thread(StackTraceHelperThread)); - fStackTraceHelper.helperThread->detach(); + fStackTraceHelper.fHelperThread.reset(new std::thread(StackTraceHelperThread)); + fStackTraceHelper.fHelperThread->detach(); }