Mark an HTTP/2 connection instead of looking it up - #2320

isHttp2 asked the pipeline for the multiplex handler by name, and the
write path asks isHttp2 of every request: once to decide whether to
store the per-request future on the channel, once to route the write. A
pipeline lookup walks the handlers comparing names, and an HTTP/1.1
connection, having no such handler, is walked to the end to say no,
which is the common case for anyone not using HTTP/2. In one profile of
a client with HTTP/2 disabled, DefaultChannelPipeline.context0 took 83
CPU samples on that account alone.

The multiplex handler is installed in exactly one place, so a channel
attribute is set beside it and isHttp2 reads that: a binary search over
integer keys in a small array rather than a walk with a string compare
per handler. hasAttr rather than attr().get(), which would add an entry
to the attribute map of every HTTP/1.1 channel just to find none.

Not a config check. Reading isHttp2Enabled first would be cheaper still,
but the two can disagree: NettyConnectListener upgrades on the ALPN
result alone, and a caller who supplies an SslContext or an
SslEngineFactory of their own controls what ALPN advertises whatever the
config says - which the WebSocket guard beside it already accounts for.
A request would then be written as HTTP/1.1 onto an HTTP/2 pipeline. The
attribute costs nothing extra and cannot disagree, since it is set where
the handler is.

The attribute cannot go stale either: nothing removes the multiplex
handler from a pipeline, so there is no downgrade for the two to differ
across.

ChannelManagerHttp2MarkerTest pins them together, so a later change to
the upgrade cannot set one without the other. Removing the attribute
fails that test and 46 of the 52 in BasicHttp2Test, the write path
having routed HTTP/2 connections down the HTTP/1.1 branch.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Review feedback. The previous commit added an attribute of its own to
stand for an HTTP/2 connection, but the connection already carries one:
upgradePipelineToHttp2 attaches an Http2ConnectionState ten lines below
the multiplex handler, and writeHttp2Request reads it back as its first
statement. Routing on that drops the second marker, and with it the
question of keeping two in sync.

writeRequest reads the state once and hands it to writeHttp2Request,
which no longer looks it up again, and the null check that guarded that
lookup goes too: its only caller now routes on the state being there.

Neither the state nor the multiplex handler is ever taken away by this
client, so the two agree for as long as a connection lives. They part
after Netty's own teardown removes the handlers on close, where isHttp2
keeps saying HTTP/2 and a pipeline lookup would have stopped: both
callers are behind an active-channel check, so nothing asks by then.

The test builds one ChannelManager for the class rather than one per
test - an SslContext and an event loop group each time, for state that
lives on the channel - and closes it, which it was not doing: the fork
gained file descriptors it never gave back. The stream case opened a
bare EmbeddedChannel, which has no parent and so asserted what the
first case already did; it opens a real stream child now, which pins
the part that is actually new, a stream not inheriting the connection
state of its parent.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Review feedback.

The state was attached after the frame codec and the multiplex handler,
which left an instant in which the pipeline spoke HTTP/2 and the
connection did not yet say so. That gap was harmless while a null state
meant "carry on anyway", and this branch made it load bearing: a write
landing there used to take the HTTP/2 path with an unaccounted stream
slot, and would now take the HTTP/1.1 path onto an HTTP/2 pipeline and
die in UnsupportedMessageTypeException. Nothing can reach a channel that
early today - all three callers are on its event loop with the channel
in neither the pool nor the registry - but that is a fact about the
callers rather than about this method. Attaching first makes it a
property of the upgrade, and inverts what a racing write would do: it
would open a stream on a pipeline that has no multiplex handler yet,
which fails out of Http2StreamChannelBootstrap and is retried, rather
than corrupting a connection.

The javadoc on isHttp2 claimed neither marker is ever taken away. That
is not true of the handler: Netty strips the pipeline on close, so a
closed connection answers no by lookup and yes by attribute - the
divergence this branch records elsewhere, contradicted here. It also
said the write path asks this of every request, which this branch is
what stopped being true: writeRequest reads the state itself now and
never calls isHttp2. It says what holds instead, that the two agree
while the connection is live and only the attribute survives its close.

Two javadoc blocks had ended up stacked on writeHttp2Request, so the
description of the stream child model was dropped from the published
javadoc and only the @PARAM survived. And the state != null guards
inside openHttp2Stream can no longer be false, the only caller having
routed on that state being non-null, so they go the way of the one at
the top.

Continue100Interceptor explained itself in terms of writeRequest looking
the multiplex handler up in the pipeline, which it no longer does. The
reason stands - a stream child carries neither the handler nor the state
- so only the wording changed.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>