jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.86k stars 1.91k forks source link

Jetty 12: Removal of ByteBufferPool breaks AbstractConnector subclasses #9284

Closed kohlschuetter closed 1 year ago

kohlschuetter commented 1 year ago

Jetty version(s) jetty 12.0.x commit ded18f523d7d1fbac6569256c9bb98aff89e0000

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.

sbordet commented 1 year ago

Fixed by #9300.