Skip to content

Removed ReaderWriter::readAllBytes#6070

Merged
jbescos merged 1 commit intoeclipse-ee4j:4.xfrom
mkarg:patch-2
Mar 11, 2026
Merged

Removed ReaderWriter::readAllBytes#6070
jbescos merged 1 commit intoeclipse-ee4j:4.xfrom
mkarg:patch-2

Conversation

@mkarg
Copy link
Member

@mkarg mkarg commented Feb 8, 2026

As planned for Jersey 4.0 (see notes in the removed code): Getting rid of a worarkound for a bug in one specific application server product (not a bug of Jersey, not a bug of OpenJDK). The vendor had several years of time to fix his own code. Keeping copies of OpenJDK-internals within libraries and applications is a no-go prevents JRE-internal optimizations.

Disclaimer: The contributor is part of the core-libs team at OpenJDK.

@mkarg mkarg marked this pull request as draft February 8, 2026 18:42
As planned for Jersey 4.0 (see notes in the removed code): Getting rid of a worarkound for a bug in one specific application server product (not a bug of Jersey, not a bug of OpenJDK). The vendor had several years of time to fix his own code. Keeping copies of OpenJDK-internals within libraries and applications is a no-go prevents JRE-internal optimizations.

Disclaimer: The contributor is part of the core-libs team at OpenJDK.
@mkarg mkarg marked this pull request as ready for review February 9, 2026 12:09
@mkarg
Copy link
Member Author

mkarg commented Feb 9, 2026

Kindly asking all committers to review my PR. Thanks.

Copy link
Contributor

@jansupol jansupol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am +0 about this.
Pros: less code, maybe JDK will come up with some better code than this one.
Cons: JDK by default does one condition less in the code ( if (nread == BUFFER_SIZE) { // This differs from JDK version
break; // prevents a bug (See ReaderWriterTest)
})
This one less condition breaks with some stream implementation (which are incorrect, but ...)

@mkarg mkarg self-assigned this Feb 25, 2026
Copy link
Member

@jbescos jbescos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not against this, but are there any new enhancements in readAllBytes to want this change now?. It looks we are preventing a bug in some JDKs.

On the other hand, we cannot keep one eye on this forever.

@mkarg
Copy link
Member Author

mkarg commented Mar 5, 2026

The target of this MR is to allow the JDK to change and optimize the implementation at any time, without waiting for another Jersey release. At OpenJDK we are actively working on optimizations all the time, and my target is to remove anything that prevents applications from automatically gaining the benefits just by upgrading the JVM. So unless someone vetoes with good reason, I'm all for merging this PR ASAP. As a committer, I hereby veto closing it without having seen a veto against it.

Besides that, it is not the job of any Open Source Project to workaround bugs of downstream products. In particular, without clearly naming that product and the need to do that.

@jbescos jbescos merged commit f645f4a into eclipse-ee4j:4.x Mar 11, 2026
7 checks passed
@mkarg mkarg deleted the patch-2 branch March 11, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants