eclipse-vertx / eclipse-vertx/vert.x
Type Pollution Scalability issue over Vertx/Netty HTTP 2 pipeline traversal
- Dominant language
- Java
- Stars
- 14.7k
- Forks
- 2.1k
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 30
Description
As reported (and fixed) at https://github.com/netty/netty/pull/12806, only `ChannelDuplexHandler`s in netty have been fixed to solve the infamous type pollution issue: by running an hello-world like test with HTTP 2 in quarkus and attaching https://github.com/RedHatPerf/type-pollution-agent to it, this case popup
```
2: io.vertx.core.http.impl.VertxHttp2ConnectionHandler
Count: 32816944
Types:
io.netty.handler.codec.http2.Http2Connection$Listener
io.netty.channel.ChannelInboundHandler
io.netty.channel.ChannelOutboundHandler
io.netty.handler.codec.http2.Http2LifecycleManager
Traces:
io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.addStream(DefaultHttp2Connection.java:805)
class: io.netty.handler.codec.http2.Http2Connection$Listener
count: 7912545
io.netty.handler.codec.http2.DefaultHttp2Connection.notifyClosed(DefaultHttp2Connection.java:357)
class: io.netty.handler.codec.http2.Http2Connection$Listener
count: 6228227
io.netty.channel.AbstractChannelHandlerContext.invokeRead(AbstractChannelHandlerContext.java:839)
class: io.netty.channel.ChannelOutboundHandler
count: 5488418
io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
class: io.netty.channel.ChannelInboundHandler
count: 5343287
io.netty.channel.AbstractChannelHandlerContext.invokeChannelReadComplete(AbstractChannelHandlerContext.java:486)
class: io.netty.channel.ChannelInboundHandler
count: 5172992
io.netty.handler.codec.http2.DefaultHttp2Connection.removeStream(DefaultHttp2Connection.java:317)
class: io.netty.handler.codec.http2.Http2Connection$Listener
count: 1525703
io.netty.handler.codec.http2.DefaultHttp2Connection$ActiveStreams.addToActiveStreams(DefaultHttp2Connection.java:995)
class: io.netty.handler.codec.http2.Http2Connection$Listener
count: 1145767
io.netty.channel.AbstractChannelHandlerContext.invokeChannelInactive(AbstractChannelHandlerContext.java:305)
class: io.netty.channel.ChannelInboundHandler
count: 1
io.netty.channel.ChannelHandlerMask.mask0(ChannelHandlerMask.java:123)
class: io.netty.channel.ChannelOutboundHandler
count: 1
io.netty.handler.codec.http2.DefaultHttp2Connection.goAwayReceived(DefaultHttp2Connection.java:237)
class: io.netty.handler.codec.http2.Http2Connection$Listener
count: 1
io.netty.handler.codec.http2.DefaultHttp2ConnectionEncoder.lifecycleManager(DefaultHttp2ConnectionEncoder.java:62)
class: io.netty.handler.codec.http2.Http2LifecycleManager
count: 1
io.netty.channel.ChannelHandlerMask.mask0(ChannelHandlerMask.java:94)
class: io.netty.channel.ChannelInboundHandler
count: 1
--------------------------```
```
The first one in the chart (not reported) is a false positive (meaning that we have a single concrete class in all the relevant code paths making the issue to disappear, optimized away by the JIT): this is is why I'm reporting the affected classes starting from `2`.
This case instead is real, awaiting to be triggered by "normal" and possible runtime condititions
eg
If the user/vertx setup a pipeline which contains 1 more `io.netty.channel.ChannelInboundHandler`-only or `io.netty.channel.ChannelOutboundHandler`-only types, which will hit:
```
io.netty.channel.AbstractChannelHandlerContext.invokeRead(AbstractChannelHandlerContext.java:839)
class: io.netty.channel.ChannelOutboundHandler
count: 5488418
io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
class: io.netty.channel.ChannelInboundHandler
count: 5343287
io.netty.channel.AbstractChannelHandlerContext.invokeChannelReadComplete(AbstractChannelHandlerContext.java:486)
class: io.netty.channel.ChannelInboundHandler
count: 5172992
```
other than just `io.vertx.core.http.impl.VertxHttp2ConnectionHandler`'; this will make the type checks there for this class to happen for real, causing the klass's `secondary_super_cache` to ping pong among these types, and hitting the issue at https://github.com/netty/netty/pull/12806#issuecomment-1259216902
In short, the problem arise because ` io.vertx.core.http.impl.VertxHttp2ConnectionHandler` is extending `Http2ConnectionHandler`'s which is transitively implementing both inbound/outbound handlers.

As usual; this seems a kind of fix which should happen within Netty, but I'm reporting here because is directly affecting us and Netty's `Http2ChannelDuplexHandler` TBH, being a duplex handler, doesn't seem affect (last famous words...).
Additionally, we do have other interfaces which the IntelliJ diagram seems to have forgotten (it's a bug???) ie `Http2FrameListener` and `Http2Connection.Listener` , but only the latter appear in the agent's report.
It doesn't seem harmful because the only implementor of `Http2Connection$Listener` seems `VertxHttp2ConnectionHandler` apart from an implementor within Netty which we don't use (because we don't use `Http2ChannelDuplexHandler`).
Contributor guide
Assessment
This issue has not been assessed yet.