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

Bazel@HEAD on JDK 17: GoogleTestSecurityManager WARNING: System::setSecurityManager will be removed in a future release #14502

Closed
davido opened this issue Jan 2, 2022 · 25 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Java Issues for Java rules type: bug

Comments

@davido
Copy link
Contributor

davido commented Jan 2, 2022

I'm running Gerrit tests on JDK 17 and seeing this warning:

  $ bazeldev test --config java17 //javatests/com/google/gerrit/acceptance/...
[...]
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by com.google.testing.junit.runner.util.GoogleTestSecurityManager (file:/home/davido/.cache/bazel/_bazel_davido/27a001f4182820ef315d8d2d4f1edafe/external/remote_java_tools/java_tools/Runner_deploy.jar)
WARNING: Please consider reporting this to the maintainers of com.google.testing.junit.runner.util.GoogleTestSecurityManager
WARNING: System::setSecurityManager will be removed in a future release

To reproduce, clone recursively Gerrit apply this CL: [1], and run with Bazel@HEAD (at least 5.0.0rc3):

  $ bazeldev test --config java17 //javatests/com/google/gerrit/acceptance/rest/revision:RevisionIT

Related: #11146.

[1] https://gerrit-review.googlesource.com/c/gerrit/+/291864

@sventiffe sventiffe added team-Rules-Java Issues for Java rules untriaged labels Jan 14, 2022
@sventiffe
Copy link
Contributor

@comius

@comius
Copy link
Contributor

comius commented Jan 19, 2022

cc @cushon

@comius comius added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Jan 19, 2022
@cushon
Copy link
Contributor

cushon commented Jan 19, 2022

System::setSecurityManager will be removed in a future release

This warning is being emitted by the JDK, and is the expected result of https://openjdk.java.net/jeps/411

I don't think anything here is specific to Bazel?

@fmeum
Copy link
Collaborator

fmeum commented Jan 19, 2022

I don't think anything here is specific to Bazel?

It's Bazel's TestRunner that calls System::setSecurityManager. Unless there is a way to disable the warning or replace the usage, Bazel tests with JDK 17 will probably always show this warning.

@cushon
Copy link
Contributor

cushon commented Jan 19, 2022

Of course, thanks.

We're looking at alternatives to the test runner. We may end up remove the use of SecurityManager entirely.

As far as I know there isn't a timeline for the removal yet, but we'll keep an eye on this and try to have something in place before it actually goes away.

In the interim the warnings are not very actionable, but I'm not sure they're suppressible.

IIUC there is some ongoing discussion upstream about provide alternatives to some of the supported functionality, e.g. discouraging calls to System.exit, but I'm not sure there are concrete plans there yet.

@cushon
Copy link
Contributor

cushon commented Jan 21, 2022

Reading https://openjdk.java.net/jeps/411,

@fmeum
Copy link
Collaborator

fmeum commented Jan 21, 2022

  • the warnings can be disabled (at least for now) with -Djava.security.manager=allow

Are you sure that's true? The JEP seems to imply that this flag only makes the startup warning go away, not the runtime warning when setSecurityManager is called. The relevant part of the code also doesn't seem to check the property.

@cushon
Copy link
Contributor

cushon commented Jan 21, 2022

@fmeum
Copy link
Collaborator

fmeum commented Jan 21, 2022

It does check it in these places and will not print the startup warning if it's set to allow, but I think that it will still print the four-line usage warning repeated in the issue description.

@davido
Copy link
Contributor Author

davido commented Jan 22, 2022

Reading https://openjdk.java.net/jeps/411,

  • the warnings can be disabled (at least for now) with -Djava.security.manager=allow

Thanks, @cushon. This works on Java 17: the warning is disappeared with this option.

However, it cannot be set unconditionally, because, with this option Java 11 doesn't work, as the setting is not backwards compatible:

java.lang.Error: Could not create SecurityManager
	at java.lang.System.initPhase3(java.base@11.0.12/System.java:2065)
Caused by: java.lang.ClassNotFoundException: allow
	at jdk.internal.loader.BuiltinClassLoader.loadClass(java.base@11.0.12/BuiltinClassLoader.java:581)
	at jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(java.base@11.0.12/ClassLoaders.java:178)
	at java.lang.ClassLoader.loadClass(java.base@11.0.12/ClassLoader.java:522)
	at java.lang.Class.forName0(java.base@11.0.12/Native Method)
	at java.lang.Class.forName(java.base@11.0.12/Class.java:398)
	at java.lang.System.initPhase3(java.base@11.0.12/System.java:2050)

At least for Gerrit Code Review, this diff seems to work in both cases (Java 11 and Java 17):

diff --git a/BUILD b/BUILD
index f6ec40e2aa..30d6547da5 100644
--- a/BUILD
+++ b/BUILD
@@ -4,9 +4,9 @@ load("//tools/bzl:pkg_war.bzl", "pkg_war")
 package(default_visibility = ["//visibility:public"])
 
 config_setting(
-    name = "java11",
+    name = "java17",
     values = {
-        "java_toolchain": "@bazel_tools//tools/jdk:toolchain_java11",
+        "java_language_version": "17",
     },
 )
 
diff --git a/tools/bzl/junit.bzl b/tools/bzl/junit.bzl
index 0e7ea3a7e1..0e1fa5db49 100644
--- a/tools/bzl/junit.bzl
+++ b/tools/bzl/junit.bzl
@@ -75,6 +75,10 @@ POST_JDK8_OPTS = [
     "-Djava.locale.providers=COMPAT,CLDR,SPI",
 ]
 
+POST_JDK17_OPTS = [
+    "-Djava.security.manager=allow",
+]
+
 def junit_tests(name, srcs, **kwargs):
     s_name = name.replace("-", "_") + "TestSuite"
     _gen_suite(
@@ -82,7 +86,12 @@ def junit_tests(name, srcs, **kwargs):
         srcs = srcs,
         outname = s_name,
     )
-    jvm_flags = kwargs.get("jvm_flags", []) + POST_JDK8_OPTS
+    jvm_flags = kwargs.get("jvm_flags", [])
+    jvm_flags = jvm_flags + select({
+         "//:java17": POST_JDK8_OPTS + POST_JDK17_OPTS,
+         "//conditions:default": POST_JDK8_OPTS,
+     })
+    POST_JDK8_OPTS
     java_test(
         name = name,
         test_class = s_name,

The better question is: can we somehow move the fix to upstream Bazel, to avoid proliferation of this diff to downstream projects?

Alone for Gerrit Code Review, we have 100+ plugins, where Bazlets Repository is used, and we also have JGit and Gitiles, that have their own Bazel build tool chains.

See also this CL: [1] for the whole fix in Gerrit.

[1] https://gerrit-review.googlesource.com/c/gerrit/+/291864

@cushon
Copy link
Contributor

cushon commented Jan 25, 2022

I agree it would be good to solve this in Bazel.

Medium term we may want to use the API for preventing calls to System.exit if https://bugs.openjdk.java.net/browse/JDK-8199704 happens, or else we could consider removing this security manager.

It would be nice to have a short term fix, and automatically passing -Djava.security.manager=allow to java_test when targeting 17 seems like a reasonable option. I think it should probably be based on --java_runtime_version, though? It's valid to configure e.g. compiling at the Java 11 language level and then running the tests on JDK 17.

@davido
Copy link
Contributor Author

davido commented Jan 25, 2022

I think it should probably be based on --java_runtime_version, though?

Right. However, at least for RBE to work with remote JDK, we need to pass --java_runtime_version=remotejdk_17:

# Builds using remotejdk_17, executes using remotejdk_17
build:java17 --java_language_version=17
build:java17 --java_runtime_version=remotejdk_17
build:java17 --tool_java_language_version=17
build:java17 --tool_java_runtime_version=remotejdk_17

So that config_setting should be defined as:

config_setting(
     name = "java17",
     values = {
        "java_runtime_version": "remotejdk_17",
     },
 )

@Jonpez2
Copy link
Contributor

Jonpez2 commented May 14, 2022

As of jdk 18, this has become a hard failure. What are the current thoughts on upstreaming the ‘allow’ setting conditional on jdk runtime >= 17? Or is there some other approach being considered?

Thanks!

@fmeum
Copy link
Collaborator

fmeum commented May 14, 2022

An alternative could be to have the test runner attach a Java agent that modifies the behavior of System.exit using ByteBuddy or ASM.

@cushon
Copy link
Contributor

cushon commented May 14, 2022

To confirm, isn't it a failure by default in 18 that can still be disabled by passing -Djava.security.manager=allow?

Internally we are currently passing that flag and waiting to see if https://bugs.openjdk.java.net/browse/JDK-8199704 happens before the API is removed.

Another option being considered is to remove the test SecurityManager sooner and rely on static analysis to prevent calls to System.exit.

@Jonpez2
Copy link
Contributor

Jonpez2 commented May 16, 2022

Right, true, it can be worked around by everyone doing that. I was just wondering whether in the interim, instead of everyone c&p'ing that into various bits of their code, it would be possible to handle it upstream.

@cushon
Copy link
Contributor

cushon commented May 16, 2022

I think it makes sense to add a shared workaround to Bazel in the interim. I'm not sure what the cleanest way only pass -Djava.security.manager=allow to JDK versions that recognize it is, though.

@jwhpryor
Copy link

I recently ran into this issue with JNI Bind and your solution worked!

That said, it would be great to have a solution that's future proof. Is this expected to be fixed in Bazel itself? I would think this would effect all Bazel users that use java_test?

Sorry for the basic question, I didn't think I'd done anything special in my setup, but I'm not very competent with setting this stuff up so I'm wondering if I'm doing something non-standard.

Thanks!

@cushon
Copy link
Contributor

cushon commented Nov 14, 2022

I think it makes sense to add a shared workaround to Bazel in the interim. I'm not sure what the cleanest way only pass -Djava.security.manager=allow to JDK versions that recognize it is, though.

This is another example where something like java_runtime(..., feature_version = 11) would be helpful.

Related: #6354 (comment)

benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 14, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)
benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 14, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)
benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 14, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)
benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 14, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)
benjaminp added a commit to benjaminp/bazel that referenced this issue Mar 14, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)
@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Mar 23, 2023
@keertk
Copy link
Member

keertk commented Mar 23, 2023

@bazel-io fork 6.2.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Mar 23, 2023
ShreeM01 pushed a commit to ShreeM01/bazel that referenced this issue Mar 24, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes bazelbuild#17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6
ShreeM01 pushed a commit to ShreeM01/bazel that referenced this issue Mar 28, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes bazelbuild#17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6
hvadehra pushed a commit that referenced this issue Mar 29, 2023
1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes #17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

(cherry picked from commit 7556e11)
hvadehra added a commit that referenced this issue Mar 31, 2023
As suggested in Use @argument files in the Java binary wrapper script #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes Bazel@HEAD on JDK 17: GoogleTestSecurityManager WARNING: System::setSecurityManager will be removed in a future release #14502.

To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes #17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6
hvadehra pushed a commit that referenced this issue Apr 3, 2023
1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes #17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

(cherry picked from commit 7556e11)
meteorcloudy pushed a commit that referenced this issue Apr 3, 2023
* Add version to JavaRuntimeInfo.

1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes #17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

(cherry picked from commit 7556e11)

* Use string.replace() instead of string.format() to avoid issues with select{} in the template string

* Remove test and fix formatting

* Fix starlark_repository_test

Change the grep pattern to exactly match what the test is looking for. The old pattern would match BUILD file content from the jdk_build_file.bzl template

---------

Co-authored-by: Benjamin Peterson <benjamin@engflow.com>
copybara-service bot pushed a commit to google/guice that referenced this issue May 17, 2023
* Bump minimum JRE to 11 (from 8).
* Change testing matrix from {8,11,15,17} to {11,17,21-ea}. (Except the bazel build has trouble parsing "21-ea" as a java version, so s/21-ea/20 for bazel.)
* Also test on macos (but only with jdk17). (Attempted to test on windows too, but there's issues right now.)
* Cleanup the POMs a little bit (remove unused mailing list reference, fix CI link)
* Bump bazel version to 6.2.0, otherwise the tests fail b/c they try to set a security manager (see bazelbuild/bazel#14502, which was fixed in bazelbuild/bazel@7556e11, which was cherrypicked into the 6.2.0 release). This also required suppressing the "BanJNDI" errorprone check in the JNDI extension, which was also added to 6.2.0. (A future change should probably just remove the JNDI extension altogether.)

PiperOrigin-RevId: 532797192
copybara-service bot pushed a commit to google/guice that referenced this issue May 17, 2023
* Bump minimum JRE to 11 (from 8).
* Change testing matrix from {8,11,15,17} to {11,17,21-ea}. (Except the bazel build has trouble parsing "21-ea" as a java version, so s/21-ea/20 for bazel.)
* Also test on macos (but only with jdk17). (Attempted to test on windows too, but there's issues right now.)
* Cleanup the POMs a little bit (remove unused mailing list reference, fix CI link)
* Bump bazel version to 6.2.0, otherwise the tests fail b/c they try to set a security manager (see bazelbuild/bazel#14502, which was fixed in bazelbuild/bazel@7556e11, which was cherrypicked into the 6.2.0 release). This also required suppressing the "BanJNDI" errorprone check in the JNDI extension, which was also added to 6.2.0. (A future change should probably just remove the JNDI extension altogether.)

PiperOrigin-RevId: 532797192
copybara-service bot pushed a commit to google/guice that referenced this issue May 17, 2023
* Bump minimum JRE to 11 (from 8).
* Change testing matrix from {8,11,15,17} to {11,17,21-ea}. (Except the bazel build has trouble parsing "21-ea" as a java version, so s/21-ea/20 for bazel.)
* Also test on macos (but only with jdk17). (Attempted to test on windows too, but there's issues right now.)
* Cleanup the POMs a little bit (remove unused mailing list reference, fix CI link)
* Bump bazel version to 6.2.0, otherwise the tests fail b/c they try to set a security manager (see bazelbuild/bazel#14502, which was fixed in bazelbuild/bazel@7556e11, which was cherrypicked into the 6.2.0 release). This also required suppressing the "BanJNDI" errorprone check in the JNDI extension, which was also added to 6.2.0. (A future change should probably just remove the JNDI extension altogether.)

PiperOrigin-RevId: 532849632
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes bazelbuild#17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6
hanwen pushed a commit to GerritCodeReview/gerrit that referenced this issue Dec 1, 2023
Bazel specific issue was resolved: [1] so that there is no need
to set this parameter to allow.

Also remove java17 configuration, because it's not used any more.

[1] bazelbuild/bazel#14502

Release-Notes: skip
Change-Id: I93f7069a20fe55f8cfdfa8ec1f046b85554361f5
jin added a commit to bazel-contrib/rules_jvm_external that referenced this issue Mar 18, 2024
simuons added a commit to bazelbuild/rules_scala that referenced this issue Mar 20, 2024
Conditionally set -Djava.security.manager=allow for jdk >= 17

rules_scala uses SecurityManager which is deprecated and fails at runtime on jdk21+ (no replacement for this yet)

Bazel's approach to solve this is conditionally add a jvm flag see bazelbuild/bazel#14502

CI builds were failing for some time because jdk was updated to 21
KevinBoyette pushed a commit to KevinBoyette/rules_scala that referenced this issue Mar 20, 2024
…uild#1555)

Conditionally set -Djava.security.manager=allow for jdk >= 17

rules_scala uses SecurityManager which is deprecated and fails at runtime on jdk21+ (no replacement for this yet)

Bazel's approach to solve this is conditionally add a jvm flag see bazelbuild/bazel#14502

CI builds were failing for some time because jdk was updated to 21
@peakschris
Copy link

I note that this issue is closed, but with Bazel 7.3.1, and jdk 17, and common --jvmopt=-Djava.security.manager=allow, I am still seeing this issue when running junit tests via bazel test:

WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by com.google.testing.junit.runner.util.GoogleTestSecurityManager (file:/D:/udu/b/xrcdxqdk/external/rules_java++toolchains+remote_java_tools/java_tools/Runner_deploy.jar)
WARNING: Please consider reporting this to the maintainers of com.google.testing.junit.runner.util.GoogleTestSecurityManager
WARNING: System::setSecurityManager will be removed in a future release

@cushon
Copy link
Contributor

cushon commented Aug 21, 2024

The warning can be ignored. Passing -Djava.security.manager=allow allows the security manager to be used, but it still logs the warning, and OpenJDK doesn't provide a way to disable that warning.

I don't think Bazel has good options here: we can't turn the warning off, and we don't want to stop using the SecurityManager until there's a replacement for its functionality (including https://bugs.openjdk.org/browse/JDK-8199704).

@peakschris
Copy link

I see, that makes sense.

Could bazel test, or rules_java, filter out the warning? It is really noisy in our test output, and I don't want all our developers to become confused.

@fmeum
Copy link
Collaborator

fmeum commented Aug 21, 2024

It may be possible to replace this functionality with the combination of TEST_PREMATURE_EXIT_FILE and the improved JDK logging for System.exit calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Java Issues for Java rules type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

11 participants