RFR: 8365588: defineClass that accepts a ByteBuffer does not work as expected [v5]

[Alan Bateman]

On Sat, 11 Oct 2025 01:47:44 GMT, Xueming Shen <sherman at openjdk.org> wrote:

>> ### Background
>> 
>> - ClassLoader.defineClass can receive class data in the form of arrays or ByteBuffers.
>> - For array-backed data (defineClass1), a defensive copy is made before passing it to JVM_DefineClassWithSource().
>> - For Direct-ByteBuffer variants (defineClass2), no defensive copy is made, which creates a risk that the underlying bytes could be modified while the JVM is processing them.
>> - Although a caller could always modify a buffer before a defensive copy is made — a race condition that cannot be completely prevented — the **_main concern_** is ensuring that the JVM never processes class bytes that are being concurrently modified.
>> 
>> ### Problem
>> 
>> - Concurrent modification risk during processing: while we cannot prevent pre-copy modifications, we **_must prevent the JVM from using class bytes that are being modified concurrently._**
>> - Performance concerns: defensive copies have a cost, especially for direct byte buffers. Making copies unnecessarily for trusted class loaders (like the built-in class loader) would hurt performance.
>> 
>> ### Solution
>> 
>> - Make a defensive copy of the direct byte-buffer only when the class loader is **NOT** a built-in/trusted class loader.
>> - For the built-in class loader, skip the copy because the JVM can guarantee that the buffer contents remain intact.
>> 
>> This approach ensures the integrity of  class bytes processes for untrusted or custom class loaders while minimizing performance impact for trusted or built-in loaders.
>> 
>> ### Benchmark
>> 
>> A JMH benchmark has been added to measure the potential cost of the defensive copy. The results indicate that the performance impact is minimal and largely insignificant.
>> 
>> **Before:**
>> 
>> 
>> Benchmark                                               Mode  Cnt     Score      Error  Units
>> ClassLoaderDefineClass.testDefineClassByteBufferDirect  avgt   15  8387.247 ± 1405.681  ns/op
>> ClassLoaderDefineClass.testDefineClassByteBufferHeap    avgt   15  8971.739 ± 1020.288  ns/op
>> Finished running test 'micro:org.openjdk.bench.java.lang.ClassLoaderDefineClass'
>> Test report is stored in /Users/xuemingshen/jdk26/build/macosx-aarch64/test-results/micro_org_openjdk_bench_java_lang_ClassLoaderDefineClass
>> 
>> 
>> **After:**
>> 
>> 
>> Benchmark                                               Mode  Cnt     Score      Error  Units
>> ClassLoaderDefineClass.testDefineClassByteBufferDirect  avgt   15  8521.881 ± 2002.011  ns/op
>> ClassLoaderDefineClass.testDefineClassByt...
>
> Xueming Shen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   test case update

test/jdk/java/lang/ClassLoader/defineClass/DefineClassDirectByteBuffer.java line 192:

> 190:     @MethodSource("bufferTypes")
> 191:     void testDefineClassWithCustomLoaderByteBuffer(int type, boolean readonly, int pos, boolean posAtLimit)
> 192:             throws Exception

Have you tried changing the method source to create a stream of the ByteBuffers to test? 

In the current version we need to add to bufferTypes(), getByteBufferWithTestClassBytes(), and maybe adding a new constant, in order to add a new buffer to test. I suspect it would be a lot simpler if method source created all the buffers (including the read-only buffers), and testDefineClassWithCustomLoaderByteBuffer just tests defineClass with that BB.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27569#discussion_r2423375236