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

[Alan Bateman]

On Mon, 13 Oct 2025 02:15:09 GMT, Xueming Shen <sherman at openjdk.org> wrote:

>> 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.
>
> Just tried to make the test code less verbose / repetitive :-) Updated accordingly.

Thank you, this is much better. One other thing that will simplify it further is to drop the posAtLimit parameter. Instead, testDefineClassWithCustomLoaderByteBuffer can capture the bb position and limit, then test their values after the defineClass, e.g.

        if (bb.isDirect() || bb.hasArray()) {
            assertEquals(originalPos, bb.position());
        } else {
            assertEquals(originalLimit, bb.position());
        }
        assertEquals(originalLimit, bb.limit());

This will test long-standing (and undocumented) behavior. I created JDK-8352583 on the spec issue. It would be too risky to change behavior and advance the position. There may be some flexibility to not advance the position for the read-only buffer case, but we would need to work through the compatibility impacts of that. The fallback is to just document long standing behavior.

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

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