Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up JNI initialization #25340

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/bootstrap/compile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ function build_jni() {

# Keep this `find` command in sync with the `srcs` of
# //src/main/native/windows:windows_jni
local srcs=$(find src/main/native/windows -name '*.cc' -o -name '*.h')
local srcs="src/main/native/common.cc $(find src/main/native/windows -name '*.cc' -o -name '*.h')"
[ -n "$srcs" ] || fail "Could not find sources for Windows JNI library"

# do not quote $srcs because we need to expand it to multiple args
Expand Down
22 changes: 22 additions & 0 deletions src/main/java/com/google/devtools/build/lib/jni/JniLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ private static void loadLibrary(String resourceName) throws IOException {
}
} catch (IOException e2) {
// Nothing else we can do. Rely on "delete on exit" to try clean things up later on.
if (dir != null) {
dir.toFile().deleteOnExit();
}
if (tempFile != null) {
tempFile.toFile().deleteOnExit();
}
}
throw e;
}
Expand Down Expand Up @@ -153,4 +159,20 @@ public static boolean isJniAvailable() {
public static Throwable getJniLoadError() {
return JNI_LOAD_ERROR;
}

/**
* Forcibly link a native method to eagerly trigger an {@link UnsatisfiedLinkError} in case of
* issues with the JNI library.
*/
public static void forceLinking() throws UnsatisfiedLinkError {
if (isJniAvailable()) {
ForceLinkingHelper.link();
}
}

private static final class ForceLinkingHelper {
private static native void link();

private ForceLinkingHelper() {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@
import com.google.devtools.build.lib.util.InterruptedFailureDetails;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.ProcessUtils;
import com.google.devtools.build.lib.util.StringEncoding;
import com.google.devtools.build.lib.util.TestType;
import com.google.devtools.build.lib.util.ThreadUtils;
Expand Down Expand Up @@ -873,7 +872,8 @@ public static void main(Iterable<Class<? extends BlazeModule>> moduleClasses, St
System.exit(batchMain(modules, args));
}
logger.atInfo().log(
"Starting Bazel server with %s, args %s", maybeGetPidString(), Arrays.toString(args));
"Starting Bazel server with pid %d, args %s",
ProcessHandle.current().pid(), Arrays.toString(args));
try {
// Run Blaze in server mode.
System.exit(serverMain(modules, OutErr.SYSTEM_OUT_ERR, args));
Expand Down Expand Up @@ -1027,8 +1027,8 @@ private static int batchMain(Iterable<BlazeModule> modules, String[] args) {
InterruptSignalHandler signalHandler = captureSigint(getSlowInterruptMessageSuffix(modules));
CommandLineOptions commandLineOptions = splitStartupOptions(modules, args);
logger.atInfo().log(
"Running Bazel in batch mode with %s, startup args %s",
maybeGetPidString(), commandLineOptions.getStartupArgs());
"Running Bazel in batch mode with pid %d, startup args %s",
ProcessHandle.current().pid(), commandLineOptions.getStartupArgs());

BlazeRuntime runtime;
InvocationPolicy policy;
Expand Down Expand Up @@ -1275,7 +1275,9 @@ private static BlazeRuntime newRuntime(
PathFragment outputBase = startupOptions.outputBase;
PathFragment realExecRootBase = outputBase.getRelative(ServerDirectories.EXECROOT);

maybeForceJNIByGettingPid(installBase); // Must be before first use of JNI.
// Force JNI linking before the first real use of JNI to emit a helpful error message now that
// we have the install base path handy.
forceJniLinking(installBase);

// From the point of view of the Java program --install_base, --output_base, --output_user_root,
// and --failure_detail_out are mandatory options, despite the comment in their declarations.
Expand Down Expand Up @@ -1462,32 +1464,17 @@ private static AbruptExitException createFilesystemExitException(
e);
}

private static String maybeGetPidString() {
Integer pid = maybeForceJNIByGettingPid(null);
return pid == null ? "" : "pid " + pid + " and ";
}

/** Loads JNI libraries, if necessary under the current platform. */
@Nullable
private static Integer maybeForceJNIByGettingPid(@Nullable PathFragment installBase) {
return JniLoader.isJniAvailable() ? getPidUsingJNI(installBase) : null;
}

// Force JNI linking at a moment when we have 'installBase' handy, and print
// an informative error if it fails.
private static int getPidUsingJNI(@Nullable PathFragment installBase) {
/**
* Loads and links JNI libraries, if necessary under the current platform, and prints an
* informative error to stderr if that fails.
*/
private static void forceJniLinking(PathFragment installBase) {
try {
return ProcessUtils.getpid(); // force JNI initialization
JniLoader.forceLinking();
} catch (UnsatisfiedLinkError t) {
System.err.println(
"JNI initialization failed: "
+ t.getMessage()
+ ". "
+ "Possibly your installation has been corrupted"
+ (installBase == null
? ""
: "; if this problem persists, try 'rm -fr " + installBase + "'")
+ ".");
System.err.printf(
"JNI initialization failed: %s. Possibly your installation has been corrupted; if this problem persists, try 'rm -fr %s'.\n",
t.getMessage(), installBase);
throw t;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,6 @@ private ProcessUtils() {}
*/
public static native int getgid();

/**
* Native wrapper around POSIX getpid(2) syscall.
*
* @return the process ID of this process.
*/
public static native int getpid();

/**
* Native wrapper around POSIX getuid(2).
*
Expand Down
17 changes: 3 additions & 14 deletions src/main/java/com/google/devtools/build/lib/util/ProcessUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@
package com.google.devtools.build.lib.util;

import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.windows.WindowsProcesses;

/**
* OS Process related utilities.
*
* <p>Use {@link ProcessHandle#current()} and {@link ProcessHandle#pid()} to get the PID of the
* current process.
*/
@ThreadSafe
public final class ProcessUtils {
Expand All @@ -39,19 +41,6 @@ public static int getgid() {
}
}

/**
* @return the process ID of this process.
* @throws UnsatisfiedLinkError when JNI is not available.
*/
public static int getpid() {
// TODO(ulfjack): Use ProcessHandle.current().getPid() here.
if (OS.getCurrent() == OS.WINDOWS) {
return WindowsProcesses.getpid();
} else {
return com.google.devtools.build.lib.unix.ProcessUtils.getpid();
}
}

/**
* @return the real user ID of the current process.
* @throws UnsatisfiedLinkError when JNI is not available.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.devtools.build.lib.windows;

import com.google.devtools.build.lib.jni.JniLoader;
import java.io.InputStream;

/** Process management on Windows. */
public class WindowsProcesses {
Expand Down Expand Up @@ -145,7 +146,4 @@ public static long createProcess(
public static native String processGetLastError(long process);

public static native String streamGetLastError(long process);

/** Returns the PID of the current process. */
public static native int getpid();
}
13 changes: 13 additions & 0 deletions src/main/native/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ cc_library(
],
)

cc_library(
name = "common",
srcs = [
"common.cc",
":jni.h",
":jni_md.h",
],
includes = ["."], # For jni headers.
visibility = ["//src/main/native:__subpackages__"],
alwayslink = 1,
)

cc_library(
name = "blake3_jni",
srcs = [
Expand Down Expand Up @@ -99,6 +111,7 @@ cc_binary(
visibility = ["//src/main/java/com/google/devtools/build/lib/jni:__pkg__"],
deps = [
":blake3_jni",
":common",
":latin1_jni_path",
"//src/main/cpp/util:logging",
"//src/main/cpp/util:md5",
Expand Down
22 changes: 22 additions & 0 deletions src/main/native/common.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2025 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/native/jni.h"

namespace blaze_jni {

extern "C" JNIEXPORT void JNICALL
Java_com_google_devtools_build_lib_jni_JniLoader_00024ForceLinkingHelper_link(JNIEnv *env) {}

} // namespace blaze_jni
10 changes: 0 additions & 10 deletions src/main/native/process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,6 @@ Java_com_google_devtools_build_lib_unix_ProcessUtils_getgid(JNIEnv *env, jclass
return getgid();
}

/*
* Class: com.google.devtools.build.lib.unix.ProcessUtils
* Method: getpid
* Signature: ()I
*/
extern "C" JNIEXPORT jint JNICALL
Java_com_google_devtools_build_lib_unix_ProcessUtils_getpid(JNIEnv *env, jclass clazz) {
return getpid();
}

/*
* Class: com.google.devtools.build.lib.unix.ProcessUtils
* Method: getuid
Expand Down
1 change: 1 addition & 0 deletions src/main/native/windows/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ cc_binary(
":lib-file",
":lib-process",
"//src/main/native:blake3_jni",
"//src/main/native:common",
],
)

Expand Down
6 changes: 0 additions & 6 deletions src/main/native/windows/processes-jni.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,6 @@ static std::wstring ToString(const T& e) {
return s.str();
}

extern "C" JNIEXPORT jint JNICALL
Java_com_google_devtools_build_lib_windows_WindowsProcesses_getpid(
JNIEnv* env, jclass clazz) {
return GetCurrentProcessId();
}

class JavaByteArray {
public:
JavaByteArray(JNIEnv* env, jbyteArray java_array)
Expand Down