On Tue, 30 Sep 2025 07:54:20 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.testDefineClassByteBufferHeap avgt 15 9002.842 ±
> 1099.635 ns/op
> Finished running test 'mi...
src/java.base/share/classes/java/lang/ClassLoader.java line 1075:
> 1073: }
> 1074:
> 1075: private Class<?> defineClass(String name, ByteBuffer b, int len,
> ProtectionDomain pb) {
Is an additional method really needed?
Couldn't we just add a new local `ByteBuffer` reference, point it to either `b`
(if trusted) or the newly allocated BB if not, and continue as before, passing
the new reference to `defineClass2()`?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27569#discussion_r2392365094
