diff options
Diffstat (limited to 'src/main')
11 files changed, 138 insertions, 48 deletions
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 635d672d2d..6db042054e 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -324,11 +324,6 @@ static vector<string> GetArgumentArray() { result.push_back(blaze::ConvertPath( blaze_util::JoinPath(real_install_dir, globals->extracted_binaries[0]))); - if (globals->options.grpc_port != -1) { - result.push_back("--grpc_port"); - result.push_back(ToString(globals->options.grpc_port)); - } - if (!globals->options.batch) { result.push_back("--max_idle_secs"); result.push_back(ToString(globals->options.max_idle_secs)); @@ -337,6 +332,11 @@ static vector<string> GetArgumentArray() { // the code expects it to be at args[0] if it's been set. result.push_back("--batch"); } + + if (globals->options.grpc_port != -1) { + result.push_back("--grpc_port=" + ToString(globals->options.grpc_port)); + } + result.push_back("--install_base=" + blaze::ConvertPath(globals->options.install_base)); result.push_back("--install_md5=" + globals->install_md5); @@ -468,10 +468,6 @@ static void Daemonize() { open("/dev/null", O_WRONLY); } dup(STDOUT_FILENO); // stderr (2>&1) - - // Keep server from inheriting a useless fd. - // The file lock was already lost at fork(). - close(globals->lockfd); } // Do a chdir into the workspace, and die if it fails. @@ -648,7 +644,7 @@ bool AfUnixBlazeServer::Connect() { if (fcntl(server_socket, F_SETFD, FD_CLOEXEC) == -1) { pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, - "fcntl(F_SETFD, FD_CLOEXEC failed)"); + "fcntl(F_SETFD, FD_CLOEXEC) failed"); } } @@ -1612,13 +1608,22 @@ static void CheckEnvironment() { // lock is inherited with the file descriptor across execve(), but not fork(). // So in the batch case, the JVM holds the lock until exit; otherwise, this // program holds it until exit. -static void AcquireLock() { - globals->lockfd = open(globals->lockfile.c_str(), O_CREAT|O_RDWR, 0644); - if (globals->lockfd < 0) { +void AcquireLock() { + int lockfd = open(globals->lockfile.c_str(), O_CREAT|O_RDWR, 0644); + + if (lockfd < 0) { pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, "cannot open lockfile '%s' for writing", globals->lockfile.c_str()); } + // Keep server from inheriting a useless fd if we are not in batch mode + if (!globals->options.batch) { + if (fcntl(lockfd, F_SETFD, FD_CLOEXEC) == -1) { + pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, + "fcntl(F_SETFD) failed for lockfile"); + } + } + struct flock lock; lock.l_type = F_WRLCK; lock.l_whence = SEEK_SET; @@ -1628,7 +1633,7 @@ static void AcquireLock() { lock.l_len = 4096; // Try to take the lock, without blocking. - if (fcntl(globals->lockfd, F_SETLK, &lock) == -1) { + if (fcntl(lockfd, F_SETLK, &lock) == -1) { if (errno != EACCES && errno != EAGAIN) { pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, "unexpected result from F_SETLK"); @@ -1637,7 +1642,7 @@ static void AcquireLock() { // We didn't get the lock. Find out who has it. struct flock probe = lock; probe.l_pid = 0; - if (fcntl(globals->lockfd, F_GETLK, &probe) == -1) { + if (fcntl(lockfd, F_GETLK, &probe) == -1) { pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, "unexpected result from F_GETLK"); } @@ -1656,7 +1661,7 @@ static void AcquireLock() { // Try to take the lock again (blocking). int r; do { - r = fcntl(globals->lockfd, F_SETLKW, &lock); + r = fcntl(lockfd, F_SETLKW, &lock); } while (r == -1 && errno == EINTR); fprintf(stderr, "\n"); if (r == -1) { @@ -1669,13 +1674,13 @@ static void AcquireLock() { } // Identify ourselves in the lockfile. - ftruncate(globals->lockfd, 0); + ftruncate(lockfd, 0); const char *tty = ttyname(STDIN_FILENO); // NOLINT (single-threaded) string msg = "owner=" + globals->options.GetProductName() + " launcher\npid=" + ToString(getpid()) + "\ntty=" + (tty ? tty : "") + "\n"; // Don't bother checking for error, since it's unlikely and unimportant. // The contents are currently meant only for debugging. - write(globals->lockfd, msg.data(), msg.size()); + write(lockfd, msg.data(), msg.size()); } static void SetupStreams() { @@ -1794,15 +1799,19 @@ int main(int argc, const char *argv[]) { const string self_path = GetSelfPath(); ComputeBaseDirectories(self_path); - AcquireLock(); - - WarnFilesystemType(globals->options.output_base); - EnsureFiniteStackLimit(); - blaze_server = globals->options.grpc_port >= 0 ? static_cast<BlazeServer *>(new GrpcBlazeServer()) : static_cast<BlazeServer *>(new AfUnixBlazeServer()); + if (globals->options.grpc_port < 0 || globals->options.batch) { + // The gRPC server can handle concurrent commands just fine. However, we + // need to be careful not to start two parallel instances in batch mode. + AcquireLock(); + } + + WarnFilesystemType(globals->options.output_base); + EnsureFiniteStackLimit(); + ExtractData(self_path); EnsureCorrectRunningVersion(blaze_server); KillRunningServerIfDifferentStartupOptions(blaze_server); @@ -1929,6 +1938,8 @@ void GrpcBlazeServer::Communicate() { command_server::RunRequest request; request.set_cookie(request_cookie); + request.set_block_for_lock(globals->options.block_for_lock); + request.set_client_description("pid=" + ToString(getpid())); for (const string& arg : arg_vector) { request.add_arg(arg); } diff --git a/src/main/cpp/blaze_globals.h b/src/main/cpp/blaze_globals.h index b09e5ebf7a..f8dc24a6a2 100644 --- a/src/main/cpp/blaze_globals.h +++ b/src/main/cpp/blaze_globals.h @@ -42,7 +42,6 @@ enum RestartReason { struct GlobalVariables { // Used to make concurrent invocations of this program safe. string lockfile; // = <output_base>/lock - int lockfd; string jvm_log_file; // = <output_base>/server/jvm.out diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java index eabb448529..707aabe259 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java @@ -17,6 +17,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.base.Predicates; +import com.google.common.base.Verify; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -34,6 +35,7 @@ import com.google.devtools.build.lib.util.AnsiStrippingOutputStream; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.LoggingUtil; import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.io.DelegatingOutErr; import com.google.devtools.build.lib.util.io.OutErr; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -65,6 +67,13 @@ import javax.annotation.Nullable; */ public class BlazeCommandDispatcher { + /** + * What to do if the command lock is not available. + */ + public enum LockingMode { + WAIT, // Wait until it is available + ERROR_OUT, // Return with an error + } // Keep in sync with options added in OptionProcessor::AddRcfileArgsAndOptions() private static final Set<String> INTERNAL_COMMAND_OPTIONS = ImmutableSet.of( "rc_source", "default_override", "isatty", "terminal_columns", "ignore_client_env", @@ -97,7 +106,8 @@ public class BlazeCommandDispatcher { } private final BlazeRuntime runtime; - + private final Object commandLock; + private String currentClientDescription = null; private OutputStream logOutputStream = null; /** @@ -116,6 +126,7 @@ public class BlazeCommandDispatcher { @VisibleForTesting public BlazeCommandDispatcher(BlazeRuntime runtime) { this.runtime = runtime; + this.commandLock = new Object(); } /** @@ -208,19 +219,13 @@ public class BlazeCommandDispatcher { * client process, or throws {@link ShutdownBlazeServerException} to * indicate that a command wants to shutdown the Blaze server. */ - int exec(List<String> args, OutErr outErr, long firstContactTime) - throws ShutdownBlazeServerException { - // Record the start time for the profiler. Do not put anything before this! - long execStartTimeNanos = runtime.getClock().nanoTime(); - - // The initCommand call also records the start time for the timestamp granularity monitor. - CommandEnvironment env = runtime.getWorkspace().initCommand(); - // Record the command's starting time for use by the commands themselves. - env.recordCommandStartTime(firstContactTime); - + int exec(List<String> args, OutErr outErr, LockingMode lockingMode, String clientDescription, + long firstContactTime) throws ShutdownBlazeServerException, InterruptedException { + Preconditions.checkNotNull(clientDescription); if (args.isEmpty()) { // Default to help command if no arguments specified. args = HELP_COMMAND; } + String commandName = args.get(0); // Be gentle to users who want to find out about Blaze invocation. @@ -234,8 +239,52 @@ public class BlazeCommandDispatcher { "Command '%s' not found. Try '%s help'.", commandName, Constants.PRODUCT_NAME)); return ExitCode.COMMAND_LINE_ERROR.getNumericExitCode(); } + + + synchronized (commandLock) { + if (currentClientDescription != null) { + switch (lockingMode) { + case WAIT: + outErr.printErrLn("Another command (" + currentClientDescription + ") is running. " + + " Waiting for it to complete..."); + commandLock.wait(); + break; + + case ERROR_OUT: + outErr.printErrLn(String.format("Another command (" + currentClientDescription + ") is " + + "running. Exiting immediately.")); + return ExitCode.COMMAND_LINE_ERROR.getNumericExitCode(); + + default: + throw new IllegalStateException(); + } + } + Verify.verify(currentClientDescription == null); + currentClientDescription = clientDescription; + } + + try { + return execExclusively(args, outErr, firstContactTime, commandName, command); + } finally { + synchronized (commandLock) { + currentClientDescription = null; + commandLock.notify(); + } + } + } + + private int execExclusively(List<String> args, OutErr outErr, long firstContactTime, + String commandName, BlazeCommand command) throws ShutdownBlazeServerException { Command commandAnnotation = command.getClass().getAnnotation(Command.class); + // Record the start time for the profiler. Do not put anything before this! + long execStartTimeNanos = runtime.getClock().nanoTime(); + + // The initCommand call also records the start time for the timestamp granularity monitor. + CommandEnvironment env = runtime.getWorkspace().initCommand(); + // Record the command's starting time for use by the commands themselves. + env.recordCommandStartTime(firstContactTime); + AbruptExitException exitCausingException = null; for (BlazeModule module : runtime.getBlazeModules()) { try { @@ -410,12 +459,14 @@ public class BlazeCommandDispatcher { } /** - * For testing ONLY. Same as {@link #exec(List, OutErr, long)}, but automatically uses the current - * time. + * For testing ONLY. Same as {@link #exec(List, OutErr, boolean, String, long)}, but automatically + * uses the current time. */ @VisibleForTesting - public int exec(List<String> args, OutErr originalOutErr) throws ShutdownBlazeServerException { - return exec(args, originalOutErr, runtime.getClock().currentTimeMillis()); + public int exec(List<String> args, LockingMode lockingMode, String clientDescription, + OutErr originalOutErr) throws ShutdownBlazeServerException, InterruptedException { + return exec(args, originalOutErr, LockingMode.ERROR_OUT, clientDescription, + runtime.getClock().currentTimeMillis()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index 9d612693d4..2004ffe64a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java @@ -49,6 +49,7 @@ import com.google.devtools.build.lib.query2.AbstractBlazeQueryEnvironment; import com.google.devtools.build.lib.query2.QueryEnvironmentFactory; import com.google.devtools.build.lib.query2.output.OutputFormatter; import com.google.devtools.build.lib.rules.test.CoverageReportActionFactory; +import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.LockingMode; import com.google.devtools.build.lib.runtime.commands.BuildCommand; import com.google.devtools.build.lib.runtime.commands.CanonicalizeCommand; import com.google.devtools.build.lib.runtime.commands.CleanCommand; @@ -820,9 +821,12 @@ public final class BlazeRuntime { try { LOG.info(getRequestLogString(commandLineOptions.getOtherArgs())); return dispatcher.exec(commandLineOptions.getOtherArgs(), OutErr.SYSTEM_OUT_ERR, - runtime.getClock().currentTimeMillis()); + LockingMode.ERROR_OUT, "batch client", runtime.getClock().currentTimeMillis()); } catch (BlazeCommandDispatcher.ShutdownBlazeServerException e) { return e.getExitStatus(); + } catch (InterruptedException e) { + // This is almost main(), so it's okay to just swallow it. We are exiting soon. + return ExitCode.INTERRUPTED.getNumericExitCode(); } finally { runtime.shutdown(); dispatcher.shutdown(); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/Command.java b/src/main/java/com/google/devtools/build/lib/runtime/Command.java index 83819eb9a4..c415f0a4cf 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/Command.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/Command.java @@ -117,5 +117,4 @@ public @interface Command { * accept several argument types, they can be combined with |, e.g <code>label|path</code>. */ String completion() default ""; - } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java index 659f4b7136..f1560b4ba5 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java @@ -40,11 +40,12 @@ public class CommandExecutor implements ServerCommand { } @Override - public int exec(List<String> args, OutErr outErr, long firstContactTime) { + public int exec(List<String> args, OutErr outErr, BlazeCommandDispatcher.LockingMode lockingMode, + String clientDescription, long firstContactTime) throws InterruptedException { LOG.info(BlazeRuntime.getRequestLogString(args)); try { - return dispatcher.exec(args, outErr, firstContactTime); + return dispatcher.exec(args, outErr, lockingMode, clientDescription, firstContactTime); } catch (BlazeCommandDispatcher.ShutdownBlazeServerException e) { if (e.getCause() != null) { StringWriter message = new StringWriter(); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java index c23cbf2d72..68df6a7258 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java @@ -247,4 +247,12 @@ public class CommonCommandOptions extends OptionsBase { category = "hidden", help = "Enable processing of +<file> parameters.") public boolean allowProjectFiles; + + @Option(name = "block_for_lock", + defaultValue = "true", + category = "hidden", + help = "If set (the default), a command will block if there is another one running. If " + + "unset, these commands will immediately return with an error.") + public boolean blockForLock; + } diff --git a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java index 58f0487873..8509a25389 100644 --- a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java +++ b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.server; +import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.LockingMode; import com.google.devtools.build.lib.runtime.CommandExecutor; import com.google.devtools.build.lib.server.CommandProtos.CancelRequest; import com.google.devtools.build.lib.server.CommandProtos.CancelResponse; @@ -235,7 +236,8 @@ public class GrpcServerImpl extends RPCServer implements CommandServerGrpc.Comma @Override public void run( RunRequest request, StreamObserver<RunResponse> observer) { - if (!request.getCookie().equals(requestCookie)) { + if (!request.getCookie().equals(requestCookie) + || request.getClientDescription().isEmpty()) { observer.onNext(RunResponse.newBuilder() .setExitCode(ExitCode.LOCAL_ENVIRONMENTAL_ERROR.getNumericExitCode()) .build()); @@ -251,13 +253,19 @@ public class GrpcServerImpl extends RPCServer implements CommandServerGrpc.Comma new RpcOutputStream(observer, command.id, StreamType.STDOUT), new RpcOutputStream(observer, command.id, StreamType.STDERR)); - exitCode = commandExecutor.exec(request.getArgList(), rpcOutErr, clock.currentTimeMillis()); + exitCode = commandExecutor.exec( + request.getArgList(), rpcOutErr, + request.getBlockForLock() ? LockingMode.WAIT : LockingMode.ERROR_OUT, + request.getClientDescription(), clock.currentTimeMillis()); + } catch (InterruptedException e) { + exitCode = ExitCode.INTERRUPTED.getNumericExitCode(); + commandId = ""; // The default value, the client will ignore it } // There is a chance that a cancel request comes in after commandExecutor#exec() has finished // and no one calls Thread.interrupted() to receive the interrupt. So we just reset the // interruption state here to make these cancel requests not have any effect outside of command - // execution. + // execution (after the try block above, the cancel request won't find the thread to interrupt) Thread.interrupted(); RunResponse response = RunResponse.newBuilder() diff --git a/src/main/java/com/google/devtools/build/lib/server/RPCService.java b/src/main/java/com/google/devtools/build/lib/server/RPCService.java index ebaa8b76ad..ab40553b31 100644 --- a/src/main/java/com/google/devtools/build/lib/server/RPCService.java +++ b/src/main/java/com/google/devtools/build/lib/server/RPCService.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.server; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.LockingMode; import com.google.devtools.build.lib.util.io.OutErr; import java.util.List; @@ -62,7 +63,11 @@ public final class RPCService { } String command = Iterables.getFirst(request, ""); if (appCommand != null && command.equals("blaze")) { // an application request - int result = appCommand.exec(request.subList(1, request.size()), outErr, firstContactTime); + // Blocking is done in the client for AF_UNIX communications, so if blockForLock would block, + // something went wrong + int result = appCommand.exec( + request.subList(1, request.size()), outErr, LockingMode.ERROR_OUT, "AF_UNIX client", + firstContactTime); if (appCommand.shutdown()) { // an application shutdown request shutdown(); } diff --git a/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java b/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java index f10689b319..577cb4871f 100644 --- a/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java +++ b/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.server; +import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher; import com.google.devtools.build.lib.util.io.OutErr; import java.util.List; @@ -27,7 +28,8 @@ public interface ServerCommand { * Executes the request, writing any output or error messages into err. * Returns 0 on success; any other value or exception indicates an error. */ - int exec(List<String> args, OutErr outErr, long firstContactTime) throws Exception; + int exec(List<String> args, OutErr outErr, BlazeCommandDispatcher.LockingMode lockingMode, + String clientDescription, long firstContactTime) throws InterruptedException; /** * The implementation returns true from this method to initiate a shutdown. diff --git a/src/main/protobuf/command_server.proto b/src/main/protobuf/command_server.proto index 87f74b2aef..eef2422c5e 100644 --- a/src/main/protobuf/command_server.proto +++ b/src/main/protobuf/command_server.proto @@ -26,6 +26,8 @@ message RunRequest { // This must be the request cookie from the output base. A rudimentary form of authentication. string cookie = 1; repeated string arg = 2; + bool block_for_lock = 3; // If false, the client won't wait for another client to finish + string client_description = 4; } message RunResponse { |