eclipse-vertx / eclipse-vertx/vert.x

Type Pollution Scalability issue over Vertx/Netty HTTP 2 pipeline traversal

Open
#5,039 2 comments 0 reactions 0 assignees View on GitHub
bug
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.

![image](https://github.com/eclipse-vertx/vert.x/assets/13125299/31d04767-573b-46d0-be55-ac8aad1c8dce)

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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.