Jetty 9.4.33 breaks binary compatibility of Request.setAsyncSupported()

[josephlbarnett]

Jetty version
9.4.33
Java version
11
OS type/version
linux / ubuntu 20.10
Description
We use jetty through dropwizard, and using the latest versions of both (jetty 9.4.33 / dw 2.0.14) are seeing this exception break all requests:

java.lang.NoSuchMethodError: 'void org.eclipse.jetty.server.Request.setAsyncSupported(boolean, java.lang.String)'
	at io.dropwizard.jetty.NonblockingServletHolder.handle(NonblockingServletHolder.java:57)
	at org.eclipse.jetty.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1633)
	at io.dropwizard.servlets.ThreadNameFilter.doFilter(ThreadNameFilter.java:35)
	at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
	at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1609)
	at io.dropwizard.jersey.filter.AllowedMethodsFilter.handle(AllowedMethodsFilter.java:47)
	at io.dropwizard.jersey.filter.AllowedMethodsFilter.doFilter(AllowedMethodsFilter.java:41)
	at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
	at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1609)

It looks like the signature changed to void org.eclipse.jetty.server.Request.setAsyncSupported(boolean, Object), and looks like a recompile/release of dropwizard against jetty 9.4.33 will fix the issue, but jetty's backwards binary compatibility appears to have broken here.

[sbordet]

@olamy can you add DropWizard to the list of "external" projects that we build?

[joakime]

What is the purpose of io.dropwizard.jetty.NonblockingServletHolder?

Asking as it feels like we have support for this concept already built-in.

Also, be aware of the ServletHolder.WrapFunction (and the same concept in FilterHolder and ListenerHolder as well)

[gregw]

The method org.eclipse.jetty.server.Request.setAsyncSupported is very much an internal implementation detail of jetty and it is not expected that third party code should need to call that method.

See comments on c40b955#commitcomment-43538925 but in summary the DropWizard class that calls this API appears to be an attempted optimisation to avoid lock contention within getServlet. It was ported from Scala 9 years ago and I don't know when it was originally written. I very much doubt that the optimisation is really needed, and even if it was, there are ways to achieve the outcome without depending on internal APIs (yes I know those are not clearly identified - that is a problem). See #5498 where we will review getServlet to see if we can avoid the lock.

I think that DropWizard could drop that entire class... but in the short term to make it work, the fix would be to remove the override of the handle method. It is no longer necessary for handle to call the setAsyncSupported method as we do that differently now.

Actually there are other problems with that class, the passed servlet is never initialised, nor introspected etc. It would be far better to actually called getServlet at least once and to cache the response, probably after a call to super.doStart.

Or better yet, just remove the class and hassle us if you see any lock contention.

I will close this issue as we are not going to revert this API.