On Sat, 11 Oct 2025 01:47:44 GMT, Xueming Shen <[email protected]> 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
