Remove DisableIntrinsic for currentThread

[devinrsmith]

Fixes #2500

[devinrsmith]

Noticed during #5294

I think a sufficient amount of time has passed where we can change the defaults and assume the runtime is using a JVM 11.0.17+, 17.0.5+. In cases where that is not true, the user can add -XX:DisableIntrinsic=_currentThread back in.

[devinrsmith]

I think this may be a moot point, b/c afaict, this settings to only be affecting development, and not hitting etc/dh-default.vmoptions as part of our native application release. This may have been an oversight on our part, or it may be the case that we accidentally changed something that caused its removal, or already partially removed it without closing #2500.

[niloc132]

Given how hard this was to track down, I'd hate to subject any downstream user to it. Could we test the current java version to warn if a version is found that could be affected by this, and instruct them to add their own vm options?

I can't imagine this will be the last time we hit a JVM bug, so it would come in handy in the future as well...

[devinrsmith]

I originally sketched out something that would have printed out the recommended arguments based on JVM version; devinrsmith@f9ffc27. This would introduce an extra process on startup, and I never took it further.

An alternative would be a warning on startup as you suggest @niloc132. A stricter version of this might even throw an error and stop the startup. I might prefer the stricter variant, with some sort of option to downgrade it to a warning.

All that said, I don't think we are actually applying the disable intrinsic today. I could go back through the history to see if there was a partial or accidental remove of it...

[devinrsmith]

Recalling a bit, I think it was kept out of the start script, as the primary consumer of that was our docker image layer (where we deployed a specific version of java and could apply the workaround only if necessary).

Wrt starting the JVM from embedded-server / python, we could use the java-utilities tool to find out the java version, similar to how we look up java.home, jpy-consortium/jpy@b16180c, or start small bootstrap program like devinrsmith@f9ffc27.

[chipkent]

I would lean to the strict version or auto-inject the JVM arg based on version. Most users don't look at the logs closely unless something goes really bad, and even then they just look at logs local to the error message.

[devinrsmith]

I've added a safety check layer that runs on startup, and have tested it natively and from py-embedded-server contexts successfully. There is the ability to disable all safety checks, or safety checks individually.

I've also added what may be seen as a controversial JDK_END_OF_LIFE safety check; this is a safety check that is conditioned based on the current date. If we think this is too aggressive, we could relax EOL by some fixed amount (6 months?), or delete it. I suspect that DHE would always want to disable it.

[devinrsmith]

Wrt the intrinsic, this is what it looks like running from the native application (would be very similar in docker as well):

$ ./server/jetty-app/build/install/server-jetty/bin/start
1 compiler directives added
VM settings:
    Max. Heap Size (Estimated): 29.50G
    Using VM: OpenJDK 64-Bit Server VM

# Starting io.deephaven.server.jetty.JettyMain
deephaven.cacheDir=/home/devin/.cache/deephaven
deephaven.configDir=/home/devin/.config/deephaven
deephaven.dataDir=/home/devin/.local/share/deephaven
Exception in thread "main" java.lang.IllegalStateException: The current JDK is vulnerable to the bug https://bugs.openjdk.org/browse/JDK-8287432. We recommend updating to 11.0.17+, 17.0.5+, or 21+. If that is not possible, you can apply a workaround '-XX:+UnlockDiagnosticVMOptions -XX:DisableIntrinsic=_currentThread'. To disable this safety check (not recommended), you can set the system property `-Ddeephaven.safetyCheck.JDK_8287432=false` or environment variable `DEEPHAVEN_SAFETY_CHECK_JDK_8287432=false`. To disable all safety checks (not recommended), you can set the system property `-Ddeephaven.safetyChecks=false` or environment variable `DEEPHAVEN_SAFETY_CHECKS=false`.
        at io.deephaven.server.runner.SafetyChecks.exception(SafetyChecks.java:82)
        at io.deephaven.server.runner.SafetyChecks$JDK_8287432.check(SafetyChecks.java:99)
        at io.deephaven.server.runner.SafetyChecks.check(SafetyChecks.java:42)
        at io.deephaven.server.runner.MainHelper.init(MainHelper.java:111)
        at io.deephaven.server.jetty.JettyMain.main(JettyMain.java:20)

Here's what it looks like from embedded python:

python
Python 3.12.2 (main, Feb 21 2024, 00:00:00) [GCC 13.2.1 20231205 (Red Hat 13.2.1-6)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from deephaven_server import Server
>>> s = Server()
1 compiler directives added
# Starting io.deephaven.python.server.EmbeddedServer
deephaven.cacheDir=/home/devin/.cache/deephaven
deephaven.configDir=/home/devin/.config/deephaven
deephaven.dataDir=/home/devin/.local/share/deephaven
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/tmp/server/lib64/python3.12/site-packages/deephaven_server/server.py", line 55, in __init__
    self.j_server = jpy.get_type('io.deephaven.python.server.EmbeddedServer')(host, port, dh_args)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: java.lang.IllegalStateException: The current JDK is vulnerable to the bug https://bugs.openjdk.org/browse/JDK-8287432. Please update to 11.0.17+, 17.0.5+, or 21+. If that is not possible, you can apply a workaround '-XX:+UnlockDiagnosticVMOptions -XX:DisableIntrinsic=_currentThread'. To disable this safety check (not recommended), you can set the system property `-Ddeephaven.safetyCheck.JDK_8287432=false` or environment variable `DEEPHAVEN_SAFETY_CHECK_JDK_8287432=false`. To disable all safety checks (not recommended), you can set the system property `-Ddeephaven.safetyChecks=false` or environment variable `DEEPHAVEN_SAFETY_CHECKS=false`.
        at io.deephaven.server.runner.SafetyChecks.exception(SafetyChecks.java:78)
        at io.deephaven.server.runner.SafetyChecks$Jdk8287432.check(SafetyChecks.java:95)
        at io.deephaven.server.runner.SafetyChecks.check(SafetyChecks.java:38)
        at io.deephaven.server.runner.MainHelper.init(MainHelper.java:111)
        at io.deephaven.python.server.EmbeddedServer.<init>(EmbeddedServer.java:88)

>>> 

[devinrsmith]

I've added a 1-year grace period post EOL, which seems like a more reasonable default.

[niloc132]

Trying to avoid any bikeshedding... so I approved as-is. If we end up adding a few more of these I might have some thoughts, but this will definitely do the job.

[devinrsmith]

I've removed JDK_END_OF_LIFE and ticket for follow-up if desired.

[rcaudy]

Did you test the safety checks on a vulnerable version and on current to ensure the messages are printed appropriately?

[devinrsmith]

Did you test the safety checks on a vulnerable version and on current to ensure the messages are printed appropriately?

Yes

[devinrsmith]

Adding documentation needed, mainly as key for DHE consideration.

[deephaven-internal]

Labels indicate documentation is required. Issues for documentation have been opened:

Community: deephaven/deephaven-docs-community#184