diff options
Diffstat (limited to 'src/main/cpp')
-rw-r--r-- | src/main/cpp/blaze.cc | 39 | ||||
-rw-r--r-- | src/main/cpp/blaze_util.cc | 21 | ||||
-rw-r--r-- | src/main/cpp/blaze_util.h | 3 | ||||
-rw-r--r-- | src/main/cpp/blaze_util_darwin.cc | 6 | ||||
-rw-r--r-- | src/main/cpp/blaze_util_freebsd.cc | 6 | ||||
-rw-r--r-- | src/main/cpp/blaze_util_linux.cc | 91 | ||||
-rw-r--r-- | src/main/cpp/blaze_util_mingw.cc | 8 | ||||
-rw-r--r-- | src/main/cpp/blaze_util_platform.h | 11 |
8 files changed, 83 insertions, 102 deletions
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 018e64a7af..47a87910a0 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -1021,8 +1021,9 @@ static void StartServerAndConnect(BlazeServer *server) { // disaster. int server_pid = GetServerPid(server_dir); if (server_pid > 0) { - if (KillServerProcess(server_pid, globals->options.output_base, - globals->options.install_base)) { + if (VerifyServerProcess(server_pid, globals->options.output_base, + globals->options.install_base) && + KillServerProcess(server_pid)) { fprintf(stderr, "Killed non-responsive server process (pid=%d)\n", server_pid); } @@ -1438,16 +1439,38 @@ static void EnsureCorrectRunningVersion(BlazeServer* server) { } } +// A signal-safe version of fprintf(stderr, ...). +// +// WARNING: any output from the blaze client may be interleaved +// with output from the blaze server. In --curses mode, +// the Blaze server often erases the previous line of output. +// So, be sure to end each such message with TWO newlines, +// otherwise it may be erased by the next message from the +// Blaze server. +// Also, it's a good idea to start each message with a newline, +// in case the Blaze server has written a partial line. +static void sigprintf(const char *format, ...) { + char buf[1024]; + va_list ap; + va_start(ap, format); + int r = vsnprintf(buf, sizeof buf, format, ap); + va_end(ap); + if (write(STDERR_FILENO, buf, r) <= 0) { + // We don't care, just placate the compiler. + } +} + // Signal handler. static void handler(int signum) { + int saved_errno = errno; + switch (signum) { case SIGINT: if (++globals->sigint_count >= 3) { sigprintf("\n%s caught third interrupt signal; killed.\n\n", globals->options.product_name.c_str()); if (globals->server_pid != -1) { - KillServerProcess(globals->server_pid, globals->options.output_base, - globals->options.install_base); + KillServerProcess(globals->server_pid); } _exit(1); } @@ -1473,6 +1496,8 @@ static void handler(int signum) { kill(globals->server_pid, SIGQUIT); break; } + + errno = saved_errno; } // Constructs the command line for a server request. @@ -2026,8 +2051,10 @@ void GrpcBlazeServer::KillRunningServer() { while (reader->Read(&response)) {} // Kill the server process for good measure. - KillServerProcess(globals->server_pid, globals->options.output_base, - globals->options.install_base); + if (VerifyServerProcess(globals->server_pid, globals->options.output_base, + globals->options.install_base)) { + KillServerProcess(globals->server_pid); + } connected_ = false; } diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index 7bf95cbade..a1cf14a225 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -176,7 +176,7 @@ int MakeDirectories(const string& path, mode_t mode) { } // Replaces 'contents' with contents of 'fd' file descriptor. -// Returns false on error. Can be called from a signal handler. +// Returns false on error. bool ReadFileDescriptor(int fd, string *content) { content->clear(); char buf[4096]; @@ -193,7 +193,7 @@ bool ReadFileDescriptor(int fd, string *content) { } // Replaces 'content' with contents of file 'filename'. -// Returns false on error. Can be called from a signal handler. +// Returns false on error. bool ReadFile(const string &filename, string *content) { int fd = open(filename.c_str(), O_RDONLY); if (fd == -1) return false; @@ -435,21 +435,4 @@ void ReleaseLock(BlazeLock* blaze_lock) { close(blaze_lock->lockfd); } -// WARNING: any output from the blaze client may be interleaved -// with output from the blaze server. In --curses mode, -// the Blaze server often erases the previous line of output. -// So, be sure to end each such message with TWO newlines, -// otherwise it may be erased by the next message from the -// Blaze server. -// Also, it's a good idea to start each message with a newline, -// in case the Blaze server has written a partial line. -void sigprintf(const char *format, ...) { - char buf[1024]; - va_list ap; - va_start(ap, format); - int r = vsnprintf(buf, sizeof buf, format, ap); - va_end(ap); - (void) write(STDERR_FILENO, buf, r); -} - } // namespace blaze diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index 89181115e8..0472ddc9ec 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -119,9 +119,6 @@ string ToString(const T& value) { return oss.str(); } -// A signal-safe version of fprintf(stderr, ...). -void sigprintf(const char *format, ...); - } // namespace blaze #endif // BAZEL_SRC_MAIN_CPP_BLAZE_UTIL_H_ diff --git a/src/main/cpp/blaze_util_darwin.cc b/src/main/cpp/blaze_util_darwin.cc index 1f0acbd2cf..93645b12eb 100644 --- a/src/main/cpp/blaze_util_darwin.cc +++ b/src/main/cpp/blaze_util_darwin.cc @@ -186,10 +186,14 @@ string GetDefaultHostJavabase() { void WriteSystemSpecificProcessIdentifier(const string& server_dir) { } -bool KillServerProcess( +bool VerifyServerProcess( int pid, const string& output_base, const string& install_base) { // TODO(lberki): This might accidentally kill an unrelated process if the // server died and the PID got reused. + return true; +} + +bool KillServerProcess(int pid) { killpg(pid, SIGKILL); return true; } diff --git a/src/main/cpp/blaze_util_freebsd.cc b/src/main/cpp/blaze_util_freebsd.cc index 4429038258..d7b7354c71 100644 --- a/src/main/cpp/blaze_util_freebsd.cc +++ b/src/main/cpp/blaze_util_freebsd.cc @@ -155,10 +155,14 @@ string GetDefaultHostJavabase() { void WriteSystemSpecificProcessIdentifier(const string& server_dir) { } -bool KillServerProcess( +bool VerifyServerProcess( int pid, const string& output_base, const string& install_base) { // TODO(lberki): This might accidentally kill an unrelated process if the // server died and the PID got reused. + return true; +} + +bool KillServerProcess(int pid) { killpg(pid, SIGKILL); return true; } diff --git a/src/main/cpp/blaze_util_linux.cc b/src/main/cpp/blaze_util_linux.cc index bd5fc852cb..5db5cb538a 100644 --- a/src/main/cpp/blaze_util_linux.cc +++ b/src/main/cpp/blaze_util_linux.cc @@ -20,7 +20,6 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> // strerror -#include <sys/fcntl.h> #include <sys/socket.h> #include <sys/statfs.h> #include <sys/types.h> @@ -168,71 +167,34 @@ string GetDefaultHostJavabase() { return blaze_util::Dirname(blaze_util::Dirname(javac_dir)); } -// Called from a signal handler. Therefore, we can't use our usual set of -// helper functions for reading files, splitting strings and so on. -static bool GetStartTime(int pid, char* output, int output_len) { - char statfile[128]; - snprintf(statfile, sizeof(statfile), "/proc/%d/stat", pid); - int fd = open(statfile, O_RDONLY); - if (fd < 0) { - return false; - } +// Called from a signal handler! +static bool GetStartTime(const string& pid, string* start_time) { + string statfile = "/proc/" + pid + "/stat"; + string statline; - // Note that this allocates 1K on any random stack the signal handler is - // called on - char statline[1024]; - int statline_len = read(fd, statline, 1024); - close(fd); - if (statline_len < 0) { + if (!ReadFile(statfile, &statline)) { return false; } - // Field 22 is that start time of the process since system startup in jiffies. - int space_count = 0; - int space_21 = -1; - int space_22 = -1; - - for (int i = 0; i < statline_len; i++) { - if (statline[i] == ' ') { - switch (++space_count) { - case 21: - space_21 = i; - break; - - case 22: - space_22 = i; - break; - - default: - // We don't care - break; - } - } - } - - if (space_21 == -1 || space_22 == -1) { - // Invalid statline format + vector<string> stat_entries = blaze_util::Split(statline, ' '); + if (stat_entries.size() < 22) { pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, - "Format of stat file at %s is unknown", statfile); + "Format of stat file at %s is unknown", statfile.c_str()); } - int jiffies_len = space_22 - space_21 - 1; - if (jiffies_len >= output_len) { - // Not enough space in output buffer (Note that we need one extra byte - // for the terminating NUL!) - return false; - } - - strncpy(output, statline + space_21 + 1, jiffies_len); - output[jiffies_len] = 0; + // Start time since startup in jiffies. This combined with the PID should be + // unique. + *start_time = stat_entries[21]; return true; } void WriteSystemSpecificProcessIdentifier(const string& server_dir) { - char start_time[256]; - if (!GetStartTime(getpid(), start_time, 256)) { + string pid = ToString(getpid()); + + string start_time; + if (!GetStartTime(pid, &start_time)) { pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, - "Cannot get start time of process %d", getpid()); + "Cannot get start time of process %s", pid.c_str()); } string start_time_file = blaze_util::JoinPath(server_dir, "server.starttime"); @@ -245,13 +207,10 @@ void WriteSystemSpecificProcessIdentifier(const string& server_dir) { // On Linux we use a combination of PID and start time to identify the server // process. That is supposed to be unique unless one can start more processes // than there are PIDs available within a single jiffy. -// -// This looks complicated, but all it does is an open(), then read(), then -// close(), all of which are safe to call from signal handlers. -bool KillServerProcess( +bool VerifyServerProcess( int pid, const string& output_base, const string& install_base) { - char start_time[256]; - if (!GetStartTime(pid, start_time, sizeof(start_time))) { + string start_time; + if (!GetStartTime(ToString(pid), &start_time)) { // Cannot read PID file from /proc . Process died meantime, all is good. No // stale server is present. return false; @@ -262,14 +221,12 @@ bool KillServerProcess( blaze_util::JoinPath(output_base, "server/server.starttime"), &recorded_start_time); - // start time file got deleted, but PID file didn't. This is strange. - // Assume that this is an old Blaze process that doesn't know how to write - // start time files yet. - if (file_present && recorded_start_time != start_time) { - // This is a different process. - return false; - } + // If start time file got deleted, but PID file didn't, assume taht this is an + // old Blaze process that doesn't know how to write start time files yet. + return !file_present || recorded_start_time == start_time; +} +bool KillServerProcess(int pid) { // Kill the process and make sure it's dead before proceeding. killpg(pid, SIGKILL); int check_killed_retries = 10; diff --git a/src/main/cpp/blaze_util_mingw.cc b/src/main/cpp/blaze_util_mingw.cc index 069ce8bb5e..4807df4f8c 100644 --- a/src/main/cpp/blaze_util_mingw.cc +++ b/src/main/cpp/blaze_util_mingw.cc @@ -753,8 +753,14 @@ bool CompareAbsolutePaths(const string& a, const string& b) { return a_real == b_real; } -bool KillServerProcess( +bool VerifyServerProcess( int pid, const string& output_base, const string& install_base) { + // TODO(lberki): This might accidentally kill an unrelated process if the + // server died and the PID got reused. + return true; +} + +bool KillServerProcess(int pid) { HANDLE process = OpenProcess(PROCESS_TERMINATE, FALSE, pid); if (process == NULL) { // Cannot find the server process. Can happen if the PID file is stale. diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h index 5f8b3bfa79..3f4c880d2e 100644 --- a/src/main/cpp/blaze_util_platform.h +++ b/src/main/cpp/blaze_util_platform.h @@ -120,12 +120,15 @@ uint64_t AcquireLock(const string& output_base, bool batch_mode, // usual. void ReleaseLock(BlazeLock* blaze_lock); -// Kills a server process based on its output base and PID. Returns true if the -// server process was found and killed. -// This function can be called from a signal handler! -bool KillServerProcess( +// Verifies whether the server process still exists. Returns true if it does. +bool VerifyServerProcess( int pid, const string& output_base, const string& install_base); +// Kills a server process based on its PID. Returns true if the +// server process was found and killed. This function can be called from a +// signal handler! Returns true if successful. +bool KillServerProcess(int pid); + // Mark path as being excluded from backups (if supported by operating system). void ExcludePathFromBackup(const string &path); |