Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetty 12: Removal of ByteBufferPool breaks AbstractConnector subclasses #9284

Closed
kohlschuetter opened this issue Feb 1, 2023 · 5 comments · Fixed by #9300
Closed

Jetty 12: Removal of ByteBufferPool breaks AbstractConnector subclasses #9284

kohlschuetter opened this issue Feb 1, 2023 · 5 comments · Fixed by #9300
Labels
Bug For general bugs on Jetty side

Comments

@kohlschuetter
Copy link
Contributor

Jetty version(s)
jetty 12.0.x commit ded18f5

Description
The removal of ByteBufferPool and replacement with RetainableByteBufferPool in the constructor signature for AbstractConnector breaks subclasses of that class, such as junixsocket's AFSocketServerConnector

How to reproduce?
Implement a subclass of AbstractConnector with Jetty 11 as a dependency, then switch to 12.

@kohlschuetter kohlschuetter added the Bug For general bugs on Jetty side label Feb 1, 2023
@kohlschuetter
Copy link
Contributor Author

Proposal:

Option A
Rename RetainableByteBufferPool to ByteBufferPool

Option B

  1. Create a new interface, ByteBufferPool, which RetainableByteBufferPool extends.
    The new interface may be empty or have the bare minimum functionality for a generic ByteBufferPool.
  2. Add a constructor to AbstractConnector that takes that new ByteBufferPool as a parameter (which would usually be empty)

@sbordet
Copy link
Contributor

sbordet commented Feb 1, 2023

We will break many APIs in Jetty 12, and ByteBufferPool is one of those.

We won't support backwards compatibility with Jetty 11 -- it's unfortunate but Jetty 12 is a major release, with major architectural changes etc.

@gregw
Copy link
Contributor

gregw commented Feb 1, 2023

However, your Option A is something we will consider in our renaming Omnibus issue: #9072
Although the renaming is being done for getting the right names, not for backwards compatibility.

@kohlschuetter
Copy link
Contributor Author

Thank you for the heads-up @gregw.

I will root for renaming RetainableByteBufferPool to ByteBufferPool, because there's no longer any other ByteBufferPool.

This will greatly simplify supporting jetty from junixsocket.

kohlschuetter added a commit to kohlschuetter/jetty.project that referenced this issue Feb 2, 2023
With issue jetty#9166, ByteBufferPool was removed and replaced by
RetainableByteBufferPool. Since ByteBufferPool was used by
AbstractConnector, this change broke backwards compatibility with
third-party connectors such as junixsocket-jetty.

Since there's no longer any other ByteBufferPool, rename the
RetainableByteBufferPool interface, and thereby not only reinstate
compatibility with existing third-party libraries but also save a few
keystrokes.

jetty#9284

Signed-off-by: Christian Kohlschütter <[email protected]>
@joakime joakime linked a pull request Feb 2, 2023 that will close this issue
kohlschuetter added a commit to kohlschuetter/jetty.project that referenced this issue Feb 3, 2023
With issue jetty#9166, ByteBufferPool was removed and replaced by
RetainableByteBufferPool. Since ByteBufferPool was used by
AbstractConnector, this change broke backwards compatibility with
third-party connectors such as junixsocket-jetty.

Since there's no longer any other ByteBufferPool, rename the
RetainableByteBufferPool interface, and thereby not only reinstate
compatibility with existing third-party libraries but also save a few
keystrokes.

jetty#9284

Signed-off-by: Christian Kohlschütter <[email protected]>
kohlschuetter added a commit to kohlschuetter/jetty.project that referenced this issue Feb 4, 2023
With issue jetty#9166, ByteBufferPool was removed and replaced by
RetainableByteBufferPool. Since ByteBufferPool was used by
AbstractConnector, this change broke backwards compatibility with
third-party connectors such as junixsocket-jetty.

Since there's no longer any other ByteBufferPool, rename the
RetainableByteBufferPool interface, and thereby not only reinstate
compatibility with existing third-party libraries but also save a few
keystrokes.

jetty#9284

Signed-off-by: Christian Kohlschütter <[email protected]>
@sbordet
Copy link
Contributor

sbordet commented Feb 15, 2023

Fixed by #9300.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants