Move remaining global variables into ServerProcessInfo
The remaining variables were related to a server process and its configuration,
so move to a new class that reflects that, and make it a member of BlazeServer
so that it's not mutated in random places.
I left ServerProcessInfo in its own file since I think the next step would be
to lift all server process management out of BlazeServer and into this class.
Note that the global BlazeServer is still a thing, and the way we pass
ServerProcessInfo through to signal handlers kind of relies on it. I don't
think this is much worse than the status quo, and it shouldn't be so bad to
clean up.
RELNOTES: None
PiperOrigin-RevId: 258677286
diff --git a/src/main/cpp/BUILD b/src/main/cpp/BUILD
index 8fe3e77..7de595d 100644
--- a/src/main/cpp/BUILD
+++ b/src/main/cpp/BUILD
@@ -17,7 +17,7 @@
name = "blaze_util",
srcs = [
"blaze_util.cc",
- "global_variables.h",
+ "server_process_info.h",
"startup_options.h",
] + select({
"//src/conditions:darwin": [
@@ -94,8 +94,8 @@
srcs = [
"blaze.cc",
"blaze.h",
- "global_variables.cc",
- "global_variables.h",
+ "server_process_info.cc",
+ "server_process_info.h",
"main.cc",
] + select({
"//src/conditions:windows": ["resources.o"],
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 014aa13..8066e49 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -55,8 +55,8 @@
#include "src/main/cpp/archive_utils.h"
#include "src/main/cpp/blaze_util.h"
#include "src/main/cpp/blaze_util_platform.h"
-#include "src/main/cpp/global_variables.h"
#include "src/main/cpp/option_processor.h"
+#include "src/main/cpp/server_process_info.h"
#include "src/main/cpp/startup_options.h"
#include "src/main/cpp/util/bazel_log_handler.h"
#include "src/main/cpp/util/errors.h"
@@ -245,7 +245,8 @@
const int connect_timeout_secs,
const bool batch,
const bool block_for_lock,
- const string &output_base);
+ const string &output_base,
+ const string &server_jvm_out);
~BlazeServer();
// Acquire a lock for the server running in this output base. Returns the
@@ -279,6 +280,9 @@
// connected state.
void Cancel();
+ // Returns information about the actual server process and its configuration.
+ const ServerProcessInfo& ProcessInfo() const { return process_info_; }
+
private:
BlazeLock blaze_lock_;
bool connected_;
@@ -304,6 +308,7 @@
void SendAction(CancelThreadAction action);
void SendCancelMessage();
+ ServerProcessInfo process_info_;
const int connect_timeout_secs_;
const bool batch_;
const bool block_for_lock_;
@@ -312,13 +317,12 @@
////////////////////////////////////////////////////////////////////////
// Global Variables
-static GlobalVariables *globals;
static BlazeServer *blaze_server;
-// TODO(laszlocsomor) 2016-11-24: release the `globals` and `blaze_server`
-// objects. Currently nothing deletes them. Be careful that some functions may
-// call exit(2) or _exit(2) (attributed with ATTRIBUTE_NORETURN) meaning we have
-// to delete the objects before those.
+// TODO(laszlocsomor) 2016-11-24: release the `blaze_server` object. Currently
+// nothing deletes it. Be careful that some functions may call exit(2) or
+// _exit(2) (attributed with ATTRIBUTE_NORETURN) meaning we have to delete the
+// objects before those.
uint64_t BlazeServer::AcquireLock() {
return blaze::AcquireLock(output_base_,
@@ -752,15 +756,15 @@
std::this_thread::sleep_until(next_attempt_time);
if (!server_startup->IsStillAlive()) {
option_processor.PrintStartupOptionsProvenanceMessage();
- if (globals->jvm_log_file_append) {
+ if (server->ProcessInfo().jvm_log_file_append_) {
// Don't dump the log if we were appending - the user should know where
// to find it, and who knows how much content they may have accumulated.
BAZEL_LOG(USER) << "Server crashed during startup. See "
- << globals->jvm_log_file;
+ << server->ProcessInfo().jvm_log_file_;
} else {
BAZEL_LOG(USER) << "Server crashed during startup. Now printing "
- << globals->jvm_log_file;
- WriteFileToStderrOrDie(globals->jvm_log_file.c_str());
+ << server->ProcessInfo().jvm_log_file_;
+ WriteFileToStderrOrDie(server->ProcessInfo().jvm_log_file_.c_str());
}
exit(blaze_exit_code::INTERNAL_ERROR);
}
@@ -840,7 +844,8 @@
BlazeServerStartup *server_startup;
const int server_pid = ExecuteDaemon(
server_exe, server_exe_args, PrepareEnvironmentForJvm(),
- globals->jvm_log_file, globals->jvm_log_file_append,
+ server->ProcessInfo().jvm_log_file_,
+ server->ProcessInfo().jvm_log_file_append_,
GetEmbeddedBinariesRoot(startup_options.install_base), server_dir,
startup_options, &server_startup);
@@ -1177,7 +1182,7 @@
// Check for the case when the workspace directory deleted and then gets
// recreated while the server is running
- string server_cwd = GetProcessCWD(globals->server_pid);
+ string server_cwd = GetProcessCWD(server->ProcessInfo().server_pid_);
// If server_cwd is empty, GetProcessCWD failed. This notably occurs when
// running under Docker because then readlink(/proc/[pid]/cwd) returns
// EPERM.
@@ -1203,7 +1208,8 @@
}
}
- BAZEL_LOG(INFO) << "Connected (server pid=" << globals->server_pid << ").";
+ BAZEL_LOG(INFO)
+ << "Connected (server pid=" << server->ProcessInfo().server_pid_ << ").";
// Wall clock time since process startup.
logging_info->client_startup_duration_ms =
@@ -1212,7 +1218,7 @@
SignalHandler::Get().Install(
startup_options.product_name,
startup_options.output_base,
- globals,
+ &server->ProcessInfo(),
CancelServer);
SignalHandler::Get().PropagateSignalOrExit(
server->Communicate(
@@ -1299,15 +1305,6 @@
<< "blaze_util::MakeCanonical('" << output_base
<< "') failed: " << GetLastErrorString();
}
-
- if (!startup_options->server_jvm_out.empty()) {
- globals->jvm_log_file = startup_options->server_jvm_out;
- globals->jvm_log_file_append = true;
- } else {
- globals->jvm_log_file =
- blaze_util::JoinPath(startup_options->output_base, "server/jvm.out");
- globals->jvm_log_file_append = false;
- }
}
// Prepares the environment to be suitable to start a JVM.
@@ -1430,7 +1427,8 @@
LoggingInfo *logging_info) {
blaze_server = new BlazeServer(
startup_options.connect_timeout_secs, startup_options.batch,
- startup_options.block_for_lock, startup_options.output_base);
+ startup_options.block_for_lock, startup_options.output_base,
+ startup_options.server_jvm_out);
logging_info->command_wait_duration_ms = blaze_server->AcquireLock();
BAZEL_LOG(INFO) << "Acquired the client lock, waited "
@@ -1510,8 +1508,6 @@
return blaze_exit_code::SUCCESS;
}
- globals = new GlobalVariables();
-
string cwd = GetCanonicalCwd();
LoggingInfo logging_info(CheckAndGetBinaryPath(cwd, argv[0]), start_time);
@@ -1599,8 +1595,10 @@
const int connect_timeout_secs,
const bool batch,
const bool block_for_lock,
- const string &output_base)
+ const string &output_base,
+ const string &server_jvm_out)
: connected_(false),
+ process_info_(output_base, server_jvm_out),
connect_timeout_secs_(connect_timeout_secs),
batch_(batch),
block_for_lock_(block_for_lock),
@@ -1698,7 +1696,7 @@
this->client_ = std::move(client);
connected_ = true;
- globals->server_pid = server_pid;
+ process_info_.server_pid_ = server_pid;
return true;
}
@@ -1841,18 +1839,18 @@
// Wait for the server process to terminate (if we know the server PID).
// If it does not terminate itself gracefully within 1m, terminate it.
- if (globals->server_pid > 0 &&
- !AwaitServerProcessTermination(globals->server_pid,
+ if (process_info_.server_pid_ > 0 &&
+ !AwaitServerProcessTermination(process_info_.server_pid_,
output_base_,
kPostShutdownGracePeriodSeconds)) {
if (!status.ok()) {
BAZEL_LOG(WARNING)
<< "Shutdown request failed, server still alive: (error code: "
<< status.error_code() << ", error message: '"
- << status.error_message() << "', log file: '" << globals->jvm_log_file
- << "')";
+ << status.error_message() << "', log file: '"
+ << process_info_.jvm_log_file_ << "')";
}
- KillServerProcess(globals->server_pid, output_base_);
+ KillServerProcess(process_info_.server_pid_, output_base_);
}
connected_ = false;
@@ -1865,7 +1863,7 @@
const vector<RcStartupFlag> &original_startup_options,
const LoggingInfo &logging_info) {
assert(connected_);
- assert(globals->server_pid > 0);
+ assert(process_info_.server_pid_ > 0);
vector<string> arg_vector;
if (!command.empty()) {
@@ -1971,10 +1969,10 @@
// If the server has shut down, but does not terminate itself within a 1m
// grace period, terminate it.
if (final_response.termination_expected() &&
- !AwaitServerProcessTermination(globals->server_pid,
+ !AwaitServerProcessTermination(process_info_.server_pid_,
output_base_,
kPostShutdownGracePeriodSeconds)) {
- KillServerProcess(globals->server_pid, output_base_);
+ KillServerProcess(process_info_.server_pid_, output_base_);
}
SendAction(CancelThreadAction::JOIN);
@@ -1985,12 +1983,12 @@
BAZEL_LOG(USER) << "\nServer terminated abruptly (error code: "
<< status.error_code() << ", error message: '"
<< status.error_message() << "', log file: '"
- << globals->jvm_log_file << "')\n";
+ << process_info_.jvm_log_file_ << "')\n";
return GetExitCodeForAbruptExit(output_base_);
} else if (!finished) {
BAZEL_LOG(USER)
<< "\nServer finished RPC without an explicit exit code (log file: '"
- << globals->jvm_log_file << "')\n";
+ << process_info_.jvm_log_file_ << "')\n";
return GetExitCodeForAbruptExit(output_base_);
} else if (final_response.has_exec_request()) {
const command_server::ExecRequest& request = final_response.exec_request();
diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h
index e58a5e2..e7f5c3e 100644
--- a/src/main/cpp/blaze_util_platform.h
+++ b/src/main/cpp/blaze_util_platform.h
@@ -20,8 +20,9 @@
#include <string>
#include <vector>
-#include "src/main/cpp/util/port.h"
#include "src/main/cpp/blaze_util.h"
+#include "src/main/cpp/server_process_info.h"
+#include "src/main/cpp/util/port.h"
namespace blaze {
@@ -69,7 +70,6 @@
} // namespace embedded_binaries
-struct GlobalVariables;
class StartupOptions;
class SignalHandler {
@@ -77,12 +77,15 @@
typedef void (* Callback)();
static SignalHandler& Get() { return INSTANCE; }
- GlobalVariables* GetGlobals() const { return globals_; }
+ const ServerProcessInfo* GetServerProcessInfo() const {
+ return server_process_info_;
+ }
const std::string& GetProductName() const { return product_name_; }
const std::string& GetOutputBase() const { return output_base_; }
void CancelServer() { cancel_server_(); }
- void Install(const std::string &product_name, const std::string &output_base,
- GlobalVariables* globals, Callback cancel_server);
+ void Install(
+ const std::string &product_name, const std::string &output_base,
+ const ServerProcessInfo* server_process_info, Callback cancel_server);
ATTRIBUTE_NORETURN void PropagateSignalOrExit(int exit_code);
private:
@@ -90,10 +93,10 @@
std::string product_name_;
std::string output_base_;
- GlobalVariables* globals_;
+ const ServerProcessInfo* server_process_info_;
Callback cancel_server_;
- SignalHandler() : globals_(nullptr), cancel_server_(nullptr) {}
+ SignalHandler() : server_process_info_(nullptr), cancel_server_(nullptr) {}
};
// A signal-safe version of fprintf(stderr, ...).
diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc
index eb8d2c1..c7ee7b6 100644
--- a/src/main/cpp/blaze_util_posix.cc
+++ b/src/main/cpp/blaze_util_posix.cc
@@ -44,7 +44,6 @@
#include <string>
#include "src/main/cpp/blaze_util.h"
-#include "src/main/cpp/global_variables.h"
#include "src/main/cpp/startup_options.h"
#include "src/main/cpp/util/errors.h"
#include "src/main/cpp/util/exit_code.h"
@@ -142,9 +141,9 @@
SigPrintf(
"\n%s caught third interrupt signal; killed.\n\n",
SignalHandler::Get().GetProductName().c_str());
- if (SignalHandler::Get().GetGlobals()->server_pid != -1) {
+ if (SignalHandler::Get().GetServerProcessInfo()->server_pid_ != -1) {
KillServerProcess(
- SignalHandler::Get().GetGlobals()->server_pid,
+ SignalHandler::Get().GetServerProcessInfo()->server_pid_,
SignalHandler::Get().GetOutputBase());
}
_exit(1);
@@ -164,10 +163,11 @@
signal_handler_received_signal = SIGPIPE;
break;
case SIGQUIT:
- SigPrintf("\nSending SIGQUIT to JVM process %d (see %s).\n\n",
- SignalHandler::Get().GetGlobals()->server_pid,
- SignalHandler::Get().GetGlobals()->jvm_log_file.c_str());
- kill(SignalHandler::Get().GetGlobals()->server_pid, SIGQUIT);
+ SigPrintf(
+ "\nSending SIGQUIT to JVM process %d (see %s).\n\n",
+ SignalHandler::Get().GetServerProcessInfo()->server_pid_,
+ SignalHandler::Get().GetServerProcessInfo()->jvm_log_file_.c_str());
+ kill(SignalHandler::Get().GetServerProcessInfo()->server_pid_, SIGQUIT);
break;
}
@@ -176,11 +176,11 @@
void SignalHandler::Install(const string &product_name,
const string &output_base,
- GlobalVariables* globals,
+ const ServerProcessInfo *server_process_info,
SignalHandler::Callback cancel_server) {
product_name_ = product_name;
output_base_ = output_base;
- globals_ = globals;
+ server_process_info_ = server_process_info;
cancel_server_ = cancel_server;
// Unblock all signals.
diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index 7930479..05dd373 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -38,7 +38,6 @@
#include "src/main/cpp/blaze_util.h"
#include "src/main/cpp/blaze_util_platform.h"
-#include "src/main/cpp/global_variables.h"
#include "src/main/cpp/startup_options.h"
#include "src/main/cpp/util/errors.h"
#include "src/main/cpp/util/exit_code.h"
@@ -298,9 +297,9 @@
SigPrintf(
"\n%s caught third Ctrl+C handler signal; killed.\n\n",
SignalHandler::Get().GetProductName().c_str());
- if (SignalHandler::Get().GetGlobals()->server_pid != -1) {
+ if (SignalHandler::Get().GetServerProcessInfo()->server_pid_ != -1) {
KillServerProcess(
- SignalHandler::Get().GetGlobals()->server_pid,
+ SignalHandler::Get().GetServerProcessInfo()->server_pid_,
SignalHandler::Get().GetOutputBase());
}
_exit(1);
@@ -320,11 +319,11 @@
void SignalHandler::Install(const string &product_name,
const string &output_base,
- GlobalVariables* globals,
+ const ServerProcessInfo *server_process_info_,
SignalHandler::Callback cancel_server) {
product_name_ = product_name;
output_base_ = output_base;
- globals_ = globals;
+ server_process_info_ = server_process_info_;
cancel_server_ = cancel_server;
::SetConsoleCtrlHandler(&ConsoleCtrlHandler, TRUE);
}
diff --git a/src/main/cpp/global_variables.cc b/src/main/cpp/global_variables.cc
deleted file mode 100644
index fd90f5e..0000000
--- a/src/main/cpp/global_variables.cc
+++ /dev/null
@@ -1,22 +0,0 @@
-// Copyright 2016 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "src/main/cpp/global_variables.h"
-
-namespace blaze {
-
-GlobalVariables::GlobalVariables()
- : server_pid(-1) {}
-
-} // namespace blaze
diff --git a/src/main/cpp/server_process_info.cc b/src/main/cpp/server_process_info.cc
new file mode 100644
index 0000000..ed8fa41
--- /dev/null
+++ b/src/main/cpp/server_process_info.cc
@@ -0,0 +1,37 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/main/cpp/server_process_info.h"
+
+#include "src/main/cpp/util/path.h"
+
+namespace blaze {
+
+static std::string GetJvmOutFile(
+ const std::string &output_base, const std::string &server_jvm_out) {
+ if (!server_jvm_out.empty()) {
+ return server_jvm_out;
+ } else {
+ return blaze_util::JoinPath(output_base, "server/jvm.out");
+ }
+}
+
+ServerProcessInfo::ServerProcessInfo(
+ const std::string &output_base, const std::string &server_jvm_out)
+ : jvm_log_file_(GetJvmOutFile(output_base, server_jvm_out)),
+ jvm_log_file_append_(!server_jvm_out.empty()),
+ // TODO(b/69972303): Formalize the "no server" magic value or rm it.
+ server_pid_(-1) {}
+
+} // namespace blaze
diff --git a/src/main/cpp/global_variables.h b/src/main/cpp/server_process_info.h
similarity index 65%
rename from src/main/cpp/global_variables.h
rename to src/main/cpp/server_process_info.h
index 9044a50..1a1813a 100644
--- a/src/main/cpp/global_variables.h
+++ b/src/main/cpp/server_process_info.h
@@ -11,12 +11,9 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
-//
-// global_variables.h: The global state in the blaze.cc Blaze client.
-//
-#ifndef BAZEL_SRC_MAIN_CPP_GLOBAL_VARIABLES_H_
-#define BAZEL_SRC_MAIN_CPP_GLOBAL_VARIABLES_H_
+#ifndef BAZEL_SRC_MAIN_CPP_SERVER_PROCESS_INFO_H_
+#define BAZEL_SRC_MAIN_CPP_SERVER_PROCESS_INFO_H_
#include <sys/types.h>
#include <string>
@@ -26,24 +23,26 @@
namespace blaze {
-class StartupOptions;
+// Encapsulates information around the blaze server process and its
+// configuration.
+class ServerProcessInfo final {
+ public:
+ ServerProcessInfo(
+ const std::string &output_base, const std::string &server_jvm_out);
-struct GlobalVariables {
- GlobalVariables();
-
- // Whrere to write the server's JVM's output. Default value is
- // <output_base>/server/jvm.out.
- std::string jvm_log_file;
+ // When running as a daemon, where the deamonized server's stdout and stderr
+ // should be written.
+ const std::string jvm_log_file_;
// Whether or not the jvm_log_file should be opened with O_APPEND.
- bool jvm_log_file_append;
+ const bool jvm_log_file_append_;
// TODO(laszlocsomor) 2016-11-28: move pid_t usage out of here and wherever
// else it appears. Find some way to not have to declare a pid_t here, either
// by making PID handling platform-independent or some other idea.
- pid_t server_pid;
+ pid_t server_pid_;
};
} // namespace blaze
-#endif // BAZEL_SRC_MAIN_CPP_GLOBAL_VARIABLES_H_
+#endif // BAZEL_SRC_MAIN_CPP_SERVER_PROCESS_INFO_H_
