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

[Alan Bateman]

On Tue, 30 Sep 2025 21:02:03 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 refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   8365588: defineClass that accepts a ByteBuffer does not work as expected

I think the changes to ClassLoader.defineClass are good. I created [JDK-8352583](https://bugs.openjdk.org/browse/JDK-8352583) some time ago to document whether it advances the buffer position (this is technical debt that goes back to when the method was added in JDK 5).  Right now, the only case where the positive is advanced is when called with a read-only buffer backed by an array. The proposed change preserves this, so there is no change in behavior.

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

> 30:  * @build DefineClassDirectByteBuffer
> 31:  * @run junit/othervm --add-opens java.base/java.lang=ALL-UNNAMED -Dmode=Direct DefineClassDirectByteBuffer
> 32:  * @run junit/othervm --add-opens java.base/java.lang=ALL-UNNAMED -Dmode=Heap DefineClassDirectByteBuffer

I think it would be better to use a ParameterizedTest and a method source that give you a good selection of buffers (direct, view of a memory segment, heap, read-only, ...). No need to open java.lang as the class loader in the test that use super.defineClass to invoke the protected method in the super class.

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

PR Comment: https://git.openjdk.org/jdk/pull/27569#issuecomment-3376054427
PR Review Comment: https://git.openjdk.org/jdk/pull/27569#discussion_r2410084132