Jetty 9 deflate extension and potential memory leak

[slovdahl]

Background

Jetty 9 defaults to supporting e.g. the permessage-deflate and x-webkit-deflate-frame Sec-WebSocket-Extensions. Historically, Java code using classes like java.util.zip.Deflater have been suspectible to native memory leaks, and especially if Java 8 is used. To this date, it's unclear if all these issues have been fully solved in Java 8. See e.g. these for bug reports related to Jetty and deflate:

• https://stackoverflow.com/questions/39538512/using-permessage-deflate-causing-native-memory-leak-in-jetty9-3-11
• https://bugs.eclipse.org/bugs/show_bug.cgi?id=487197
• Deflater/Inflater memory leak with WebSocket permessage-deflate extension jetty/jetty.project#293
• Implement Deflater / Inflater Object Pool jetty/jetty.project#300

We are currently experiencing what we believe is a native memory leak. We have not been able to reproduce it outside production (yet), but the current main suspect is the deflate extension.

Current Atmosphere state

Looking at AbstractJetty9AsyncSupportWithWebSocket, it seems like we try to disable all extensions during the WebSocket handshake.

In Jetty 9.3 and older, this actually had an impact, because the getExtensions() call actually returns the underlying mutable extensions list: https://github.com/eclipse/jetty.project/blob/a8e0689a08c66342b151a9e9de7a4fc1f2659d22/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeRequest.java#L86.

In Jetty 9.4, this doesn't have any effect any more, as the getExtensions() call now returns a new mutable list parsed from the underlying header each time: https://github.com/eclipse/jetty.project/blob/276905651256e973f6eac5d3fe266596aa62d6d3/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/ServletUpgradeRequest.java#L120.

The req.getExtensions().clear() call was originally introduced in 2d10aa0 to work around a bug in Jetty 9.0 that was fixed close to 10 years ago.

(FWIW, the workaround should probably have been removed many years ago, but I'm not sure if it's safe to remove it in a minor version. We could probably add a comment in the code about when it works.)

Suggestions

There are a few ways of disabling extensions, see e.g. jetty/jetty.project#1341 (comment) for some hints. However, I can't get any of those to work in our case, where Jetty 9 runs in embedded mode with Atmosphere on top. I guess we would need to hook in to Jetty here or here to be able to unregister the deflate extensions.

One option could be to add an ApplicationConfig for specifically opting out of all extensions, or a given list of extensions. Thoughts about this @jfarcand?

Atmosphere Info

• version 2.7.3
• atmosphere.js version
• extensions used

Systems (please complete the following information):

• OS: Ubuntu 18.04
• Java version and distribution: 8u312
• Server name and version: Jetty 9.4.44

[slovdahl]

It didn't take long after summarizing the effort this far until I realized there might be one workaround: a servlet filter that removes the deflate extensions from the header in incoming WebSocket handshakes. Will try this first.

[jfarcand]

@slovdahl I like your proposal with ApplicationConfig. Go for it and I can review and release 2.7.6

[slovdahl]

It might be a bit early to judge, but we deployed a new version with the servlet filter where extensions are removed from the incoming handshake about 3 hours ago, and the the initial memory usage at least went down significantly, and the constant and slow increase in memory usage is not happening yet at least. So it seems like the deflate extensions were indeed the trigger in our case.

And a little bit of backstory too: in this specific case, we were running an older version of Atmosphere and Jetty 8 until a few weeks ago. When we updated it to the latest Atmosphere and Jetty 9, it started leaking memory. We're also in the process of moving over to Java 11+, which could also have "solved" this problem, but we're not there just yet.

[slovdahl]

Disabling compression made quite a big difference (that was done at the rightmost peak):

But still using quite a bit more memory than with Jetty 8 and Atmosphere 2.2.x:

Not a huge problem at the moment, maybe that's just the way it is. We'll focus on getting it up to Java 11 and 17 now instead, that might bring some improvements in performace too.