Skip to content

Commit 818f015

Browse files
authored
Add Java 24 compatibility and remove SecurityManager support (bazel-contrib#1719)
Java 24 degrades the SecurityManager into a hollow shell that just throws exceptions. Later versions will remove the interface entirely. This commit removes references to the SecurityManager from rules_scala, which will make tests that call System.exit crash the worker. It is often possible to edit code to avoid calling System.exit, and guarding against it requires more effort in Java 24+, likely involving attaching an agent. Until someone actually has that need, it doesn't seem worth doing. Bazel is doing the same thing for now, see bazelbuild/bazel#24354 Add -Dcom.google.testing.junit.runner.shouldInstallTestSecurityManager=false for junit tests. While recent Bazel versions set this automatically, older Bazel versions do not, and since we're removing the flag that allows the SM, we need to set this to prevent crashing for people running older Bazel versions with Java 17+ Also exclude the .ijwb directory from the buildifier lint check. Failures due to those files are irritating when running the test locally.
1 parent aebd839 commit 818f015

File tree

9 files changed

+19
-71
lines changed

9 files changed

+19
-71
lines changed

scala/private/phases/phase_coverage.bzl

+1-5
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@ load(
88
"//scala/private:coverage_replacements_provider.bzl",
99
_coverage_replacements_provider = "coverage_replacements_provider",
1010
)
11-
load(
12-
"//scala/private:rule_impls.bzl",
13-
_allow_security_manager = "allow_security_manager",
14-
)
1511

1612
def phase_coverage_library(ctx, p):
1713
args = struct(
@@ -64,7 +60,7 @@ def _phase_coverage(ctx, p, srcjars):
6460
outputs = [output_jar],
6561
executable = ctx.attr._code_coverage_instrumentation_worker.files_to_run,
6662
execution_requirements = {"supports-workers": "1"},
67-
arguments = ["--jvm_flag=%s" % f for f in _allow_security_manager(ctx)] + [args],
63+
arguments = [args],
6864
)
6965

7066
replacements = {input_jar: output_jar}

scala/private/phases/phase_scalafmt.bzl

+1-5
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@
44
# Outputs to format the scala files when it is explicitly specified
55
#
66
load("//scala/private:paths.bzl", _scala_extension = "scala_extension")
7-
load(
8-
"//scala/private:rule_impls.bzl",
9-
_allow_security_manager = "allow_security_manager",
10-
)
117

128
def phase_scalafmt(ctx, p):
139
if ctx.attr.format:
@@ -26,7 +22,7 @@ def _build_format(ctx):
2622
files = []
2723
srcs = []
2824
manifest_content = []
29-
jvm_flags = ["-Dfile.encoding=UTF-8"] + _allow_security_manager(ctx)
25+
jvm_flags = ["-Dfile.encoding=UTF-8"]
3026
for src in ctx.files.srcs:
3127
# only format scala source files, not generated files
3228
if src.path.endswith(_scala_extension) and src.is_source:

scala/private/phases/phase_write_executable.bzl

+4-15
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,19 @@
1+
load("//scala/private:common.bzl", "rlocationpath_from_file")
2+
13
#
24
# PHASE: write executable
35
#
46
# DOCUMENT THIS
57
#
68
load(
79
"//scala/private:rule_impls.bzl",
8-
"allow_security_manager",
910
"expand_location",
1011
"first_non_empty",
1112
"is_windows",
1213
"java_bin",
1314
"java_bin_windows",
1415
"runfiles_root",
15-
"specified_java_runtime",
1616
)
17-
load("//scala/private:common.bzl", "rlocationpath_from_file")
1817

1918
def phase_write_executable_scalatest(ctx, p):
2019
# jvm_flags passed in on the target override scala_test_jvm_flags passed in on the
@@ -29,7 +28,7 @@ def phase_write_executable_scalatest(ctx, p):
2928
jvm_flags = [
3029
"-DRULES_SCALA_MAIN_WS_NAME=%s" % ctx.workspace_name,
3130
"-DRULES_SCALA_ARGS_FILE=%s" % rlocationpath_from_file(ctx, p.runfiles.args_file),
32-
] + expand_location(ctx, final_jvm_flags) + _allow_security_manager_for_specified_java_runtime(ctx),
31+
] + expand_location(ctx, final_jvm_flags),
3332
use_jacoco = ctx.configuration.coverage_enabled,
3433
)
3534
return _phase_write_executable_default(ctx, p, args)
@@ -44,7 +43,7 @@ def phase_write_executable_repl(ctx, p):
4443
def phase_write_executable_junit_test(ctx, p):
4544
args = struct(
4645
rjars = p.coverage_runfiles.rjars,
47-
jvm_flags = p.jvm_flags + ctx.attr.jvm_flags + _allow_security_manager_for_specified_java_runtime(ctx),
46+
jvm_flags = p.jvm_flags + ctx.attr.jvm_flags + ["-Dcom.google.testing.junit.runner.shouldInstallTestSecurityManager=false"],
4847
main_class = "com.google.testing.junit.runner.BazelTestRunner",
4948
use_jacoco = ctx.configuration.coverage_enabled,
5049
)
@@ -186,13 +185,3 @@ def _jar_path_based_on_java_bin(ctx):
186185
java_bin_var = java_bin(ctx)
187186
jar_path = java_bin_var.rpartition("/")[0] + "/jar"
188187
return jar_path
189-
190-
# Allow security manager for generated test executables if they will be run with jdk >= 17
191-
def _allow_security_manager_for_specified_java_runtime(ctx):
192-
return allow_security_manager(
193-
ctx,
194-
specified_java_runtime(
195-
ctx,
196-
default_runtime = ctx.attr._java_runtime[java_common.JavaRuntimeInfo],
197-
),
198-
)

scala/private/rule_impls.bzl

+1-10
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ def compile_scala(
124124
# TODO: scalac worker is run with @bazel_tools//tools/jdk:runtime_toolchain_type
125125
# which is different from rules_java where compilation runtime is used from
126126
# @bazel_tools//tools/jdk:toolchain_type
127-
final_scalac_jvm_flags = first_non_empty(scalac_jvm_flags, toolchain.scalac_jvm_flags) + allow_security_manager(ctx)
127+
final_scalac_jvm_flags = first_non_empty(scalac_jvm_flags, toolchain.scalac_jvm_flags)
128128

129129
ctx.actions.run(
130130
inputs = ins,
@@ -221,12 +221,3 @@ def java_bin_windows(ctx):
221221

222222
def is_windows(ctx):
223223
return ctx.configuration.host_path_separator == ";"
224-
225-
# Return a jvm flag allowing security manager for jdk runtime >= 17
226-
# If no runtime is supplied then runtime is taken from ctx.attr._java_host_runtime
227-
# This must be a runtime used in generated java_binary script (usually workers using SecurityManager)
228-
def allow_security_manager(ctx, runtime = None):
229-
java_runtime = runtime if runtime else ctx.attr._java_host_runtime[java_common.JavaRuntimeInfo]
230-
231-
# Bazel 5.x doesn't have java_runtime.version defined
232-
return ["-Djava.security.manager=allow"] if hasattr(java_runtime, "version") and java_runtime.version >= 17 else []

scala_proto/private/scala_proto_aspect.bzl

+3-4
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
1+
load("@bazel_skylib//lib:dicts.bzl", "dicts")
12
load("@rules_proto//proto:defs.bzl", "ProtoInfo")
23
load("//scala/private:common.bzl", "write_manifest_file")
34
load("//scala/private:dependency.bzl", "legacy_unclear_dependency_info_for_protobuf_scrooge")
5+
load("//scala/private:phases/api.bzl", "extras_phases", "run_aspect_phases")
46
load(
57
"//scala/private:rule_impls.bzl",
68
"compile_scala",
79
"specified_java_compile_toolchain",
8-
_allow_security_manager = "allow_security_manager",
910
)
1011
load("//scala/private/toolchain_deps:toolchain_deps.bzl", "find_deps_info_on")
1112
load(
1213
"//scala_proto/private:scala_proto_aspect_provider.bzl",
1314
"ScalaProtoAspectInfo",
1415
)
15-
load("//scala/private:phases/api.bzl", "extras_phases", "run_aspect_phases")
16-
load("@bazel_skylib//lib:dicts.bzl", "dicts")
1716

1817
def _import_paths(proto, ctx):
1918
# Under Bazel 7.x, direct_sources from generated protos may still contain
@@ -77,7 +76,7 @@ def _generate_sources(ctx, toolchain, proto):
7776

7877
ctx.actions.run(
7978
executable = toolchain.worker,
80-
arguments = ["--jvm_flag=%s" % f for f in _allow_security_manager(ctx)] + [toolchain.worker_flags, args],
79+
arguments = [toolchain.worker_flags, args],
8180
inputs = depset(transitive = [descriptors, toolchain.generators_jars]),
8281
outputs = outputs.values(),
8382
tools = [toolchain.protoc],

src/java/io/bazel/rulesscala/worker/Worker.java

-25
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@
99
import java.nio.charset.StandardCharsets;
1010
import java.nio.file.Files;
1111
import java.nio.file.Paths;
12-
import java.security.Permission;
1312
import java.util.List;
14-
import java.util.regex.Matcher;
15-
import java.util.regex.Pattern;
1613

1714
/**
1815
* A base for JVM workers.
@@ -54,15 +51,6 @@ public static void workerMain(String workerArgs[], Interface workerInterface) th
5451

5552
/** The main loop for persistent worker processes */
5653
private static void persistentWorkerMain(Interface workerInterface) {
57-
System.setSecurityManager(
58-
new SecurityManager() {
59-
@Override
60-
public void checkPermission(Permission permission) {
61-
Matcher matcher = exitPattern.matcher(permission.getName());
62-
if (matcher.find()) throw new ExitTrapped(Integer.parseInt(matcher.group(1)));
63-
}
64-
});
65-
6654
InputStream stdin = System.in;
6755
PrintStream stdout = System.out;
6856
PrintStream stderr = System.err;
@@ -94,8 +82,6 @@ public void checkPermission(Permission permission) {
9482
String[] workerArgs = stringListToArray(request.getArgumentsList());
9583
String[] args = expandArgsIfArgsfile(workerArgs);
9684
workerInterface.work(args);
97-
} catch (ExitTrapped e) {
98-
code = e.code;
9985
} catch (Exception e) {
10086
if (e instanceof Interface.WorkerException) System.err.println(e.getMessage());
10187
else e.printStackTrace();
@@ -177,17 +163,6 @@ public void reset() {
177163
}
178164
}
179165

180-
static class ExitTrapped extends RuntimeException {
181-
final int code;
182-
183-
ExitTrapped(int code) {
184-
super();
185-
this.code = code;
186-
}
187-
}
188-
189-
private static Pattern exitPattern = Pattern.compile("exitVM\\.(-?\\d+)");
190-
191166
private static String[] stringListToArray(List<String> argList) {
192167
int numArgs = argList.size();
193168
String[] args = new String[numArgs];

src/java/io/bazel/rulesscala/worker/WorkerTest.java

-3
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,6 @@ public void testBufferWriteReadAndReset() throws Exception {
214214

215215
@AfterClass
216216
public static void teardown() {
217-
// Persistent workers install a security manager. We need to
218-
// reset it here so that our own process can exit!
219-
System.setSecurityManager(null);
220217
}
221218

222219
// Copied/modified from Bazel's MoreAsserts

tools/BUILD

+6
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,19 @@ WARNINGS_CONFIG = [
1818

1919
buildifier(
2020
name = "lint_check",
21+
exclude_patterns = [
22+
"./.ijwb/*",
23+
],
2124
lint_mode = "warn",
2225
lint_warnings = WARNINGS_CONFIG,
2326
mode = "check",
2427
)
2528

2629
buildifier(
2730
name = "lint_fix",
31+
exclude_patterns = [
32+
"./.ijwb/*",
33+
],
2834
lint_mode = "fix",
2935
lint_warnings = WARNINGS_CONFIG,
3036
mode = "fix",

twitter_scrooge/twitter_scrooge.bzl

+3-4
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
load("@bazel_skylib//lib:dicts.bzl", "dicts")
12
load(
23
"//scala/private:common.bzl",
34
"write_manifest_file",
@@ -8,13 +9,11 @@ load(
89
)
910
load(
1011
"//scala/private:rule_impls.bzl",
11-
"allow_security_manager",
1212
"compile_java",
1313
"compile_scala",
1414
)
15-
load("//thrift:thrift_info.bzl", "ThriftInfo")
1615
load("//thrift:thrift.bzl", "merge_thrift_infos")
17-
load("@bazel_skylib//lib:dicts.bzl", "dicts")
16+
load("//thrift:thrift_info.bzl", "ThriftInfo")
1817

1918
def _colon_paths(data):
2019
return ":".join([f.path for f in sorted(data)])
@@ -86,7 +85,7 @@ def _generate_jvm_code(ctx, label, compile_thrifts, include_thrifts, jar_output,
8685
# be correctly handled since the executable is a jvm app that will
8786
# consume the flags on startup.
8887
#arguments = ["--jvm_flag=%s" % flag for flag in ctx.attr.jvm_flags] +
89-
arguments = ["--jvm_flag=%s" % f for f in allow_security_manager(ctx)] + ["@" + argfile.path],
88+
arguments = ["@" + argfile.path],
9089
)
9190

9291
def _compiled_jar_file(actions, scrooge_jar):

0 commit comments

Comments
 (0)