RFR: 8378786: PeerConnectionId::cloneBuffer should use absolute bulk get

[Volkan Yazici]

On Fri, 27 Feb 2026 14:26:19 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> src/java.net.http/share/classes/jdk/internal/net/http/quic/PeerConnectionId.java line 77:
>> 
>>> 75:         final byte[] idBytes = new byte[src.remaining()];
>>> 76:         src.get(src.position(), idBytes);
>>> 77:         return ByteBuffer.wrap(idBytes);
>> 
>> You can alternatively consider replacing these 3 lines with `return src.slice(src.position(), src.limit())`, I guess.
>
> Not exactly as that would retain the memory from the input buffer which might be large (could be a slice of the whole packet).

Okay, I see — would be nice to document this in a comment. It might be me, but this crucial detail is very easy to miss.

>> src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicConnectionImpl.java line 1949:
>> 
>>> 1947:             if (t instanceof AssertionError) {
>>> 1948:                 this.terminator.terminate(TerminationCause.forException(t));
>>> 1949:             }
>> 
>> Would you mind explaining the rationale for this change, please?
>
> This is to make tests fail if the assertion is fired. Typically an exception while decoding/parsing would just cause the packet to be skipped. An assertion here indicates that invariants have been violated (a problem in our own code which should never happen), not that bad/unexpected bytes were received. If that happens (assert fired), we want to know about it. Assertions are typically enabled in testing (they are when running JDK tests) but not in production.

I see. I'm assuming this is the most economic — in terms of, not just LoC, but also complexity — way to inform tests about this unexpected condition.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29956#discussion_r2864692470
PR Review Comment: https://git.openjdk.org/jdk/pull/29956#discussion_r2864692247