ENT-11328: Update node initialisation to be flexible if JDK modules are not open to reflection #7640

shamsasari · 2024-01-03T16:18:16Z

This was work-in-progress, exploratory PR

The overall goal was to eliminate, or reduce as much as possible, the need for the JVM --add-opens flags when running the unit tests. The reason for this is it will also affect users when they write tests using the mock network or the in-process node driver. The users will essentially need to copy most of the same flags the node uses when it runs. This isn't a very good user experience, though perhaps manageable with some documentation.

The process I took was to remove the node-jvm-args.txt from the jvmArgs for all unit tests, and only added for tests which absolutely needed them, such as checkpoint serialisation tests. This later example had its own module created checkpoint-tests.

The main reason the flags are needed is due to Kryo serialisation of checkpoints, when it tries to access private members of the JDK via reflection. The access is typically needed by the Kryo serialisers, so instead of adding these serialisers, it first checks if it has access to do so and then adds them. This is primarily done by the new Class<*>.isPackageOpen. If the package is open, which will be the case with a proper Corda node process, then the serialiser is added as usual. If it's not open, which will be the case with mock nodes, then it registers a fall back serialiser via Kryo.registerAsInaccessible. This will try to write out checkpoints using a backend serialiser, but it will not be able to deserialise the correct type. For example, it will serialise an unmodifiableList using a normal List serialiser, but it can't deserialise it since it needs access to internals of the unmodifiableList class. So instead during checkpoint deserialisation it will throw an exception with a help message:

Restoring checkpoints containing ${type.name} objects is not supported in this test " +
                "environment. If you wish to restore these checkpoints in your tests then use the out-of-process node driver, or add " +
                "--add-opens=${type.fullyQualifiedPackage}=ALL-UNNAMED to the test JVM args.

Since most mock node tests will not be restarting nodes, which is when checkpoints are deserialised, this will not affect majority of tests.

There are other changes to reduce the need to reflect on JDK internals.

…re not open to reflection

shamsasari force-pushed the shams-flexible-kryo-init branch from 344cd9b to a39e0d6 Compare January 3, 2024 17:06

shamsasari added 2 commits January 4, 2024 11:07

ENT-11328: Update node initialisation to be flexible if JDK modules a…

0123b14

…re not open to reflection

Investigating DuplicateSerializerLogTest

d811f6f

shamsasari mentioned this pull request Mar 1, 2024

ENT-11101: Fix all crypto issues introduced by Java 17 upgrade #7675

Merged

shamsasari force-pushed the shams-flexible-kryo-init branch from a39e0d6 to d811f6f Compare March 28, 2024 13:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENT-11328: Update node initialisation to be flexible if JDK modules are not open to reflection #7640

ENT-11328: Update node initialisation to be flexible if JDK modules are not open to reflection #7640

ENT-11328: Update node initialisation to be flexible if JDK modules are not open to reflection #7640

Are you sure you want to change the base?

ENT-11328: Update node initialisation to be flexible if JDK modules are not open to reflection #7640

Conversation