Skip to content

Commit

Permalink
Use isIPv6Preferred to check if we should use ipv6 or not
Browse files Browse the repository at this point in the history 
    Follow up on: bazelbuild/bazel#11776

    Fixes bazelbuild/bazel#11756

    RELNOTES: None
    PiperOrigin-RevId: 321541781
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 9a03d46 commit f6017af
Showing 1 changed file with 166 additions and 59 deletions.
225 changes: 166 additions & 59 deletions ...a/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@
import com.google.common.flogger.GoogleLogger;
import com.google.common.net.InetAddresses;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.common.util.concurrent.Uninterruptibles;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.runtime.BlazeCommandResult;
import com.google.devtools.build.lib.runtime.BlazeRuntime;
import com.google.devtools.build.lib.runtime.CommandDispatcher;
import com.google.devtools.build.lib.runtime.CommandDispatcher.LockingMode;
import com.google.devtools.build.lib.runtime.SafeRequestLogging;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import com.google.devtools.build.lib.server.CommandManager.RunningCommand;
import com.google.devtools.build.lib.server.CommandProtos.CancelRequest;
Expand Down Expand Up @@ -63,12 +64,18 @@
import io.netty.channel.unix.Socket;
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;

/**
* gRPC server class.
Expand Down Expand Up @@ -102,35 +109,38 @@
*/
public class GrpcServerImpl extends CommandServerGrpc.CommandServerImplBase implements RPCServer {
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
private final boolean shutdownOnLowSysMem;

public static GrpcServerImpl create(
CommandDispatcher dispatcher,
ShutdownHooks shutdownHooks,
PidFileWatcher pidFileWatcher,
Clock clock,
int port,
Path serverDirectory,
int serverPid,
int maxIdleSeconds,
boolean shutdownOnLowSysMem,
boolean idleServerTasks) {
SecureRandom random = new SecureRandom();
return new GrpcServerImpl(
dispatcher,
shutdownHooks,
pidFileWatcher,
clock,
port,
generateCookie(random, 16),
generateCookie(random, 16),
serverDirectory,
serverPid,
maxIdleSeconds,
shutdownOnLowSysMem,
idleServerTasks);
/**
* Factory class. Instantiated by reflection.
*
* <p>Used so that method calls using reflection are as simple as possible.
*/
public static class Factory implements RPCServer.Factory {
@Override
public RPCServer create(
CommandDispatcher dispatcher,
Clock clock,
int port,
Path serverDirectory,
int maxIdleSeconds,
boolean shutdownOnLowSysMem,
boolean idleServerTasks)
throws AbruptExitException {
SecureRandom random = new SecureRandom();
return new GrpcServerImpl(
dispatcher,
clock,
port,
generateCookie(random, 16),
generateCookie(random, 16),
serverDirectory,
maxIdleSeconds,
shutdownOnLowSysMem,
idleServerTasks);
}
}


@VisibleForTesting
enum StreamType {
STDOUT,
Expand Down Expand Up @@ -265,25 +275,76 @@ public void write(int byteAsInt) throws IOException {
}
}

/**
* A thread that watches if the PID file changes and shuts down the server immediately if so.
*/
private class PidFileWatcherThread extends Thread {
private boolean shuttingDown = false;

private PidFileWatcherThread() {
super("pid-file-watcher");
setDaemon(true);
}

// The synchronized block is here so that if the "PID file deleted" timer kicks in during a
// regular shutdown, they don't race.
private synchronized void signalShutdown() {
shuttingDown = true;
}

@Override
public void run() {
while (true) {
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
boolean ok = false;
try {
String pidFileContents = new String(FileSystemUtils.readContentAsLatin1(pidFile));
ok = pidFileContents.equals(pidInFile);
} catch (IOException e) {
logger.atInfo().log("Cannot read PID file: %s", e.getMessage());
// Handled by virtue of ok not being set to true
}

if (!ok) {
synchronized (PidFileWatcherThread.this) {
if (shuttingDown) {
logger.atWarning().log(
"PID file deleted or overwritten but shutdown is already in progress");
break;
}

shuttingDown = true;
// Someone overwrote the PID file. Maybe it's another server, so shut down as quickly
// as possible without even running the shutdown hooks (that would delete it)
logger.atSevere().log(
"PID file deleted or overwritten, exiting as quickly as possible");
Runtime.getRuntime().halt(ExitCode.BLAZE_INTERNAL_ERROR.getNumericExitCode());
}
}
}
}
}

// These paths are all relative to the server directory
private static final String PORT_FILE = "command_port";
private static final String REQUEST_COOKIE_FILE = "request_cookie";
private static final String RESPONSE_COOKIE_FILE = "response_cookie";
private static final String SERVER_INFO_FILE = "server_info.rawproto";

private static final AtomicBoolean runShutdownHooks = new AtomicBoolean(true);

private final CommandManager commandManager;
private final CommandDispatcher dispatcher;
private final Executor commandExecutorPool;
private final ShutdownHooks shutdownHooks;
private final Clock clock;
private final Path serverDirectory;
private final String requestCookie;
private final String responseCookie;
private final int maxIdleSeconds;
private final boolean shutdownOnLowSysMem;
private final PidFileWatcher pidFileWatcher;
private final int serverPid;
private final PidFileWatcherThread pidFileWatcherThread;
private final Path pidFile;
private final String pidInFile;
private final List<Path> filesToDeleteAtExit = new ArrayList<>();
private final int port;

private Server server;
Expand All @@ -292,29 +353,35 @@ public void write(int byteAsInt) throws IOException {
@VisibleForTesting
GrpcServerImpl(
CommandDispatcher dispatcher,
ShutdownHooks shutdownHooks,
PidFileWatcher pidFileWatcher,
Clock clock,
int port,
String requestCookie,
String responseCookie,
Path serverDirectory,
int serverPid,
int maxIdleSeconds,
boolean shutdownOnLowSysMem,
boolean doIdleServerTasks) {
this.dispatcher = dispatcher;
this.shutdownHooks = shutdownHooks;
this.pidFileWatcher = pidFileWatcher;
boolean doIdleServerTasks)
throws AbruptExitException {
Runtime.getRuntime().addShutdownHook(new Thread(() -> shutdownHook()));

// server.pid was written in the C++ launcher after fork() but before exec().
// The client only accesses the pid file after connecting to the socket
// which ensures that it gets the correct pid value.
pidFile = serverDirectory.getRelative("server.pid.txt");
try {
pidInFile = new String(FileSystemUtils.readContentAsLatin1(pidFile));
} catch (IOException e) {
throw createFilesystemFailureException(
"Server pid file read failed: " + e.getMessage(),
Code.SERVER_PID_TXT_FILE_READ_FAILURE,
e);
}
deleteAtExit(pidFile);

this.dispatcher = dispatcher;
this.clock = clock;
this.port = port;
this.requestCookie = requestCookie;
this.responseCookie = responseCookie;

this.serverDirectory = serverDirectory;
this.serverPid = serverPid;

this.port = port;
this.maxIdleSeconds = maxIdleSeconds;
this.shutdownOnLowSysMem = shutdownOnLowSysMem;
this.serving = false;
Expand All @@ -327,7 +394,11 @@ public void write(int byteAsInt) throws IOException {
.setDaemon(true)
.build()));

this.requestCookie = requestCookie;
this.responseCookie = responseCookie;

pidFileWatcherThread = new PidFileWatcherThread();
pidFileWatcherThread.start();
commandManager = new CommandManager(doIdleServerTasks);
}

Expand Down Expand Up @@ -363,8 +434,8 @@ private static String generateCookie(SecureRandom random, int byteCount) {
*/
@Override
public void prepareForAbruptShutdown() {
shutdownHooks.disable();
pidFileWatcher.signalShutdown();
disableShutdownHooks();
pidFileWatcherThread.signalShutdown();
}

@Override
Expand All @@ -388,22 +459,14 @@ public void serve() throws AbruptExitException {
throw new IOException("ipv6 is not preferred on the system.");
}
server =
NettyServerBuilder.forAddress(address)
.addService(this)
.directExecutor()
// disable auto flow control https://github.com/bazelbuild/bazel/issues/12264
.flowControlWindow(NettyServerBuilder.DEFAULT_FLOW_CONTROL_WINDOW)
.build()
.start();
NettyServerBuilder.forAddress(address).addService(this).directExecutor().build().start();
} catch (IOException ipv6Exception) {
address = new InetSocketAddress("127.0.0.1", port);
try {
server =
NettyServerBuilder.forAddress(address)
.addService(this)
.directExecutor()
// disable auto flow control https://github.com/bazelbuild/bazel/issues/12264
.flowControlWindow(NettyServerBuilder.DEFAULT_FLOW_CONTROL_WINDOW)
.build()
.start();
} catch (IOException ipv4Exception) {
Expand Down Expand Up @@ -449,7 +512,7 @@ private void writeServerStatusFiles(InetSocketAddress address) throws AbruptExit

ServerInfo info =
ServerInfo.newBuilder()
.setPid(serverPid)
.setPid(Integer.parseInt(pidInFile))
.setAddress(addressString)
.setRequestCookie(requestCookie)
.setResponseCookie(responseCookie)
Expand All @@ -463,7 +526,7 @@ private void writeServerStatusFiles(InetSocketAddress address) throws AbruptExit
}
Path serverInfoFile = serverDirectory.getChild(SERVER_INFO_FILE);
serverInfoTmpFile.renameTo(serverInfoFile);
shutdownHooks.deleteAtExit(serverInfoFile);
deleteAtExit(serverInfoFile);
} catch (IOException e) {
throw createFilesystemFailureException(
"Failed to write server info file: " + e.getMessage(), Code.SERVER_FILE_WRITE_FAILURE, e);
Expand All @@ -480,7 +543,51 @@ private void writeServerFile(String name, String contents) throws AbruptExitExce
Code.SERVER_FILE_WRITE_FAILURE,
e);
}
shutdownHooks.deleteAtExit(file);
deleteAtExit(file);
}

protected void disableShutdownHooks() {
runShutdownHooks.set(false);
}

private void shutdownHook() {
if (!runShutdownHooks.get()) {
return;
}

List<Path> files;
synchronized (filesToDeleteAtExit) {
files = new ArrayList<>(filesToDeleteAtExit);
}
for (Path path : files) {
try {
path.delete();
} catch (IOException e) {
printStack(e);
}
}
}

/**
* Schedule the specified file for (attempted) deletion at JVM exit.
*/
protected void deleteAtExit(final Path path) {
synchronized (filesToDeleteAtExit) {
filesToDeleteAtExit.add(path);
}
}

static void printStack(IOException e) {
/*
* Hopefully this never happens. It's not very nice to just write this
* to the user's console, but I'm not sure what better choice we have.
*/
StringWriter err = new StringWriter();
PrintWriter printErr = new PrintWriter(err);
printErr.println("=======[BAZEL SERVER: ENCOUNTERED IO EXCEPTION]=======");
e.printStackTrace(printErr);
printErr.println("=====================================================");
logger.atSevere().log(err.toString());
}

private void executeCommand(RunRequest request, BlockingStreamObserver<RunResponse> observer) {
Expand Down Expand Up @@ -549,7 +656,7 @@ private void executeCommand(RunRequest request, BlockingStreamObserver<RunRespon
.collect(ImmutableList.toImmutableList());

InvocationPolicy policy = InvocationPolicyParser.parsePolicy(request.getInvocationPolicy());
logger.atInfo().log(SafeRequestLogging.getRequestLogString(args));
logger.atInfo().log(BlazeRuntime.getRequestLogString(args));
result =
dispatcher.exec(
policy,
Expand Down

0 comments on commit f6017af

Please sign in to comment.