feat: add TcpStackAwareSocketChannel for z/OS TCP/IP stack recovery — GH#4776

[balhar-jakub]

Closes #4776

Adds a SocketChannel wrapper that detects z/OS TCP/IP stack restarts (EDC5122I / NetworkRecycledException) during read/write operations on accepted client sockets. When detected, the wrapper safely closes the dead socket and re-throws so Tomcat discards the connection.

Changes

• Refactored isTcpStackRestarted and isRecycledClass from instance methods to package-private static methods on TomcatAcceptFixConfig
• Added apiml.tcpStackAwareSocketChannel.enabled config property (default: true)
• Created ExcludedSocketOps interface for Lombok @Delegate exclusion list
• Created TcpStackAwareSocketChannel inner class:
  • Intercepts read(ByteBuffer), read(ByteBuffer[], int, int), write(ByteBuffer), write(ByteBuffer[], int, int)
  • Detects EDC5122I/NetworkRecycledException → log.debug + safeClose + re-throw
  • implCloseSelectableChannel and implConfigureBlocking via MethodHandles
• Wired into FixedServerSocketChannel.accept() via wrapIfEnabled() guarded by config
• 16 new unit tests (76 total pass)

Build

./gradlew :apiml-tomcat-common:clean build — BUILD SUCCESSFUL, all 76 tests pass.

(QA review + CI gate to follow)

[balhar-jakub]

QA + Security Review — PR #4792 (#4776)

Verdict: APPROVED

Build and Tests

• ./gradlew :apiml-tomcat-common:clean build — BUILD SUCCESSFUL, 76/76 tests pass
• 16 new tests added covering all read/write/safeClose/config scenarios

Pavel's Lens — All 8 Rules Checked

Rule	Status	Notes
1. Config Consistency	OK	apiml.tcpStackAwareSocketChannel.enabled has Spring default :true. Follows existing undocumented pattern (same as server.tomcat.retryRebindTimeoutSecs).
2. Deduplication	OK	isTcpStackRestarted/isRecycledClass properly extracted to static methods. Old instance method delegates. Minor: NETWORK_RECYCLED_EXCEPTION_CLASS constant on line 176 is now dead code — not used after refactor. Suggest removing in follow-up.
3. Null Safety	OK	isTcpStackRestarted checks getMessage() != null, getCause() != null, cause != t (cycle guard).
4. Test Parametrization	OK	Tests are varied enough (read/write, single/scatter, pass/fail, config on/off, integration) to not collapse cleanly into parameterized. 16 distinct test cases with good edge coverage.
5. Security Boundaries	OK	No auth/TLS/CORS/input validation changes. This is z/OS socket infrastructure. Config toggle safely disables. No secrets, no data exposure. safeClose is best-effort with catch-and-ignore.
6. z/OS Awareness	EXCELLENT	Purpose-built for z/OS TCP/IP stack recovery. Handles EDC5122I and com.ibm.net.NetworkRecycledException. Graceful degradation via config toggle.
7. Log Quality	OK	All log.debug with exception context. Messages identify operation type (read/scatter read/write/scatter write). safeClose silently swallows close errors — correct for best-effort.
8. TODO Tracking	OK	No TODO/FIXME/HACK comments found.

Minor Notes (non-blocking)

1. NETWORK_RECYCLED_EXCEPTION_CLASS constant (line 176) is dead code after the static refactor — can be cleaned up in a follow-up.
2. Instance isTcpStackRestarted wrapper (lines 277-279) delegates to static version — may still be referenced by pre-existing tests calling channel.isTcpStackRestarted(). Acceptable.

Security Assessment

• No new auth/authz boundaries introduced
• No input validation changes
• No secrets or credentials in code
• No data exposure via logs (debug level, exception context only)
• MethodHandle usage for implCloseSelectableChannel/implConfigureBlocking follows existing pattern in FixedServerSocketChannel
• @Delegate with ExcludedSocketOps correctly excludes final methods and manually overridden methods

APPROVED — ready for merge.

[balhar-jakub]

DCO check failed — all 3 commits are missing Signed-off-by lines.

Fix: git rebase --signoff v3.x.x && git push --force-with-lease

[sonarqubecloud]

Quality Gate failed

Failed conditions
78.6% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

💡 Need a hand with PR review? Try Gitar by Sonar!

[sonarqubecloud]

Quality Gate failed

Failed conditions
78.6% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[balhar-jakub]

Multi-Reviewer Review

Verdict: Approve with minor NITs.

Findings:

• ℹ️ NIT — Typo in IMPL_CLOSE_SELECTABGLE_CHANNEL_HANLE (should be HANDLE) — pre-existing, but inherited.
• ℹ️ NIT — Method counts in ExcludedSocketOps comment are slightly off (14/6 split is approximate).
• ℹ️ NIT — close() is excluded but not overridden — defense-in-depth via implCloseSelectableChannel() works.
• ℹ️ NIT — No toString() override.
• ℹ️ NIT — safeClose swallows IOException silently; consider trace-level log.
• ✅ Test coverage is excellent (every read/write variant tested, mockStatic for NetworkRecycledException).

Good:

• Feature flag apiml.tcpStackAwareSocketChannel.enabled defaults to false — safe opt-in.
• @Delegate approach correctly handles 20 excluded methods + delegates the rest.
• isTcpStackRestarted now static (was instance) — cleaner since no instance state needed.
• New tests cover both the existing isTcpStackRestarted and the new TcpStackAwareSocketChannel.

Optional follow-up:

• Update apiml-tomcat-common CHANGELOG / docs to mention the new flag.
• Consider a tenant-level integration test that toggles the flag and verifies behavior.

Required before merge: none. All findings are NIT-level polish.

[balhar-jakub]

ℹ️ NIT — two notes about TcpStackAwareSocketChannel:

1. close() is excluded from @DeleGate but not manually overridden. Looking at ExcludedSocketOps, close() is in the list (step 10) — that's because SocketChannel.close() is final in AbstractInterruptibleChannel and cannot be overridden in Java (compile error). The wrapper inherits it directly. So when close() is called, it goes through the chain AbstractInterruptableChannel.close() → implCloseSelectableChannel() (which IS overridden). Good — this means the safeClose wrapper in I/O methods is redundant with the implCloseSelectableChannel override, but it's a defense-in-depth. Worth a comment:

// close() is final on AbstractInterruptableChannel; we override implCloseSelectableChannel() instead.

2. No toString() override. The inherited Object.toString() will return class names, which is fine for debugging but may be confusing when the wrapper is logging. Consider overriding:

@Override
public String toString() {
    return "TcpStackAwareSocketChannel{delegate=" + delegate + "}";
}

Not blocking.

[balhar-jakub]

ℹ️ NIT — typo in IMPL_CLOSE_SELECTABGLE_CHANNEL_HANLE (should be HANDLE). This typo is pre-existing in the original code (line 57, 72, 184), but the PR inherits it on line 425. Worth fixing now while the file is being touched:

Suggested change

	protected void implCloseSelectableChannel() throws IOException {
	IMPL_CLOSE_SELECTABGLE_CHANNEL_HANDLE.invoke(delegate);

Cosmetic, but it improves readability.

[balhar-jakub]

ℹ️ NIT — comment counts are slightly off. The comment says "First 14 are final methods on SocketChannel; last 6 are manually overridden." Actually, SocketChannel.read(ByteBuffer) and write(ByteBuffer) are NOT final methods — they are abstract and are overridden in the wrapper. The 14/6 split is approximate and exchanges some inherited abstract methods (e.g., validOps(), provider()) with final claims. Consider rephrasing:

* The list of methods excluded from Lombok @Delegate for TcpStackAwareSocketChannel.
* Methods excluded are: (a) methods on SocketChannel that cannot be overridden via @Delegate,
* and (b) the read/write/implClose/implCloseSelectableChannel methods that are manually wrapped here.

[balhar-jakub]

✅ Good — feature flag is gated by apiml.tcpStackAwareSocketChannel.enabled (default false). This is the right default — the wrapper adds overhead per I/O call, so opt-in is the safer choice. Document the production-bringup plan in the operator docs (e.g., apiml-common-log-messages.yml or a CHANGELOG entry). What does the rollout story look like — enable per service in staging, then prod?

[balhar-jakub]

✅ Test coverage is excellent. Each variant of read/write is tested:

• pass-through (no exception)
• EDC5122I exception → close + re-throw
• non-EDC5122I exception → do NOT close
• scatter read/write variants
• NetworkRecycledException via mockStatic
• implCloseSelectableChannel / implConfigureBlocking delegation
• Configuration disabled path returns raw socket

The givenNetworkRecycledException_whenIsRecycledClass_thenReturnTrue test uses MockedStatic to mock the static method, which is the right pattern since com.ibm.net.NetworkRecycledException is not available on non-z/OS JDKs.

[balhar-jakub]

ℹ️ NIT — safeClose swallows IOException but the original delegate's close error is silently lost. This is intentional (best-effort close), but consider logging at trace level so debugging is possible:

private static void safeClose(SocketChannel ch) {
    try {
        ch.close();
    } catch (IOException e) {
        log.trace("Best-effort close failed (ignored)", e);
    }
}

Not blocking.