On Mon, 13 Oct 2025 02:15:09 GMT, Xueming Shen <[email protected]> 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
