Skip to content

Auto-port 5.0: ChannelInitializer: correct misleading comment on exceptionCaught route#16854

Merged
normanmaurer merged 1 commit into
5.0from
auto-port-pr-16847-to-5.0
May 28, 2026
Merged

Auto-port 5.0: ChannelInitializer: correct misleading comment on exceptionCaught route#16854
normanmaurer merged 1 commit into
5.0from
auto-port-pr-16847-to-5.0

Conversation

@netty-project-bot
Copy link
Copy Markdown
Contributor

Auto-port of #16847 to 5.0
Cherry-picked commit: 22d1be6


Motivation:

The catch block in ChannelInitializer.initChannel(ChannelHandlerContext) currently attributes the explicit exceptionCaught(...) call to two reasons that don't match the current code:

} catch (Throwable cause) {
    // Explicitly call exceptionCaught(...) as we removed the handler before calling initChannel(...).
    // We do so to prevent multiple calls to initChannel(...).
    exceptionCaught(ctx, cause);
} finally {
    if (!ctx.isRemoved()) {
        ctx.pipeline().remove(this);
    }
}
  • The handler is not removed before initChannel(...) is called — removal happens in the finally block immediately below.
  • Re-entrance is guarded by the initMap.add(ctx) check at the top of the method, not by this catch block.

Both clauses of the comment mislead a reader trying to understand why the catch block exists.

Modification:

Rewrite the two-line comment to name the actual mechanisms:

  • the exceptionCaught(...) call routes the failure into the pipeline;
  • re-entrance is guarded by initMap.add(ctx) above;
  • handler removal happens in the finally block below.

No code change.

Result:

Future readers of initChannel(ChannelHandlerContext) can trust the comment instead of having to cross-check it against the surrounding code.

Fixes #16846.

The same wording exists on 4.1, 4.2, and 5.0; targeting 4.1 so auto-port carries it forward.

…te (#16847)

Motivation:

The catch block in
`ChannelInitializer.initChannel(ChannelHandlerContext)` currently
attributes the explicit `exceptionCaught(...)` call to two reasons that
don't match the current code:

```java
} catch (Throwable cause) {
    // Explicitly call exceptionCaught(...) as we removed the handler before calling initChannel(...).
    // We do so to prevent multiple calls to initChannel(...).
    exceptionCaught(ctx, cause);
} finally {
    if (!ctx.isRemoved()) {
        ctx.pipeline().remove(this);
    }
}
```

- The handler is **not** removed before `initChannel(...)` is called —
removal happens in the `finally` block immediately below.
- Re-entrance is guarded by the `initMap.add(ctx)` check at the top of
the method, **not** by this catch block.

Both clauses of the comment mislead a reader trying to understand why
the catch block exists.

Modification:

Rewrite the two-line comment to name the actual mechanisms:

- the `exceptionCaught(...)` call routes the failure into the pipeline;
- re-entrance is guarded by `initMap.add(ctx)` above;
- handler removal happens in the `finally` block below.

No code change.

Result:

Future readers of `initChannel(ChannelHandlerContext)` can trust the
comment instead of having to cross-check it against the surrounding
code.

Fixes #16846.

The same wording exists on 4.1, 4.2, and 5.0; targeting 4.1 so auto-port
carries it forward.

(cherry picked from commit 22d1be6)
@normanmaurer normanmaurer added this to the 5.0.0.Final milestone May 28, 2026
@normanmaurer normanmaurer merged commit c8d08c0 into 5.0 May 28, 2026
12 of 13 checks passed
@normanmaurer normanmaurer deleted the auto-port-pr-16847-to-5.0 branch May 28, 2026 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants