Skip to content

Flaky OpenICFWebSocketTest.testRemoteConnectorInfoManager: client connection permit leak causes 5-min hang and Surefire fork timeout #102

Description

@vharseko

Symptom

build-maven (macos-latest, 26) failed on PR #101 with a Surefire fork timeout in the connector-server-grizzly module (job log):

[ERROR] org.forgerock.openicf.framework.server.OpenICFWebSocketTest. -- Time elapsed: 300.2 s <<< FAILURE!
java.util.concurrent.TimeoutException
[ERROR]   Run 4: OpenICFWebSocketTest.testRemoteConnectorInfoManager:208 » Timeout
[ERROR] Failed to execute goal ...maven-surefire-plugin:3.2.5:test on project connector-server-grizzly: There was a timeout in the fork

testRemoteConnectorInfoManager hung for exactly 300 s (the 5-minute getOrThrowUninterruptibly timeout), which alone exceeds the module's forkedProcessTimeoutInSeconds=300, so the whole fork was killed instead of reporting a single test failure. The same test passed on all other 8 matrix combinations (Linux JDK 11/17/21/25/26, macOS 11, Windows 11/26) — it is a flaky, load-sensitive test, unrelated to the PR changes.

Root cause analysis

The test waits for AsyncRemoteConnectorInfoManager.findConnectorInfoAsync(...), which only resolves after the client establishes a WS connection, completes the handshake, and receives the initial ControlRequest(CONNECTOR_INFO) response. Several defects in the client code create states from which the client never recovers, so on a slow/loaded runner the deferred promise hangs the full 5 minutes:

1. Connection permit leaked on close (fatal wedge)

ClientRemoteConnectorInfoManager.onClosed (connector-framework-server, ClientRemoteConnectorInfoManager.java:176-181):

if (availableConnectionPermitCount.get() < 1) {
    logger.ok("Destroy onClosed. Remaining permits: {0}", ...);
    return;                       // early return WITHOUT releasing the permit
}
tryReleaseConnectionPermit();

The default client has one permit. While its only connection is alive, available == 0. If that connection closes, the < 1 check is true and the permit held by the dying connection is never released. From then on:

  • the heartbeat task's connect(false) always returns null (no permits),
  • the on-demand path in trySubmitRequest is blocked by hasFreeConnectionPermit().

The client is permanently stuck with zero connections. The check is inverted — it skips the release exactly in the case where the release is needed.

2. Permit also leaked on failed TCP connect

In the CompletionHandler.failed() callback (ClientRemoteConnectorInfoManager.java:340-353) the acquired permit is not released either — a single failed initial connect wedges the client the same way.

3. Race: ControlResponse can arrive before the connection context is assigned

The initial CONNECTOR_INFO request is sent inside ClientRemoteConnectorInfoManager.handshake(...) (via WebSocketConnectionGroup.handshake, WebSocketConnectionGroup.java:114-135), but the adapter's context field is assigned only after that call returns (ClientRemoteConnectorInfoManager.java:512-515). If the server's response arrives first (different selector thread), processControlResponsesocket.getRemoteConnectionContext() returns null → NPE, the response is silently dropped, and the retry chain does not fire (it only reacts to promise failure, not to a lost response).

4. Recovery is too sparse and fragile

The only re-delivery mechanism is ConnectionManager.groupChecker (ConnectionManager.java:152-153): initial delay 1 min, period 4 min — at most two chances within the test's 300 s window, and none at all if the client is wedged per (1)/(2). Additionally trySubmitRequest(...).getPromise() in WebSocketConnectionGroup.java:125,131 NPEs if trySubmitRequest returns null (allowed by its contract), and in the retry branch that NPE is swallowed by the promise machinery.

5. Test defects

OpenICFWebSocketTest.testRemoteConnectorInfoManager (OpenICFWebSocketTest.java:207-214):

for (int i = 0; (c = manager.findConnectorInfoAsync(TEST_CONNECTOR_KEY)
        .getOrThrowUninterruptibly(5, TimeUnit.MINUTES)) == null && i < 5; i++) {
    Thread.sleep(20000);
}
  • The loop is dead code: getOrThrowUninterruptibly never returns null (it throws TimeoutException), so the poll/sleep is unreachable — the code is effectively a single 5-minute blocking wait.
  • Budget conflict: the test's 300 s wait equals forkedProcessTimeoutInSeconds=300 (connector-server-grizzly/pom.xml:163), so any single hang converts into a fork kill for the whole module.

Suggested fixes

Production code:

  1. onClosed: always release the permit held by the closing connection (remove the inverted early return).
  2. failed() callback: release the acquired permit on connect failure.
  3. Assign the adapter's context before sending the initial ControlRequest (or send it after handshake completion); guard trySubmitRequest(...) against null in WebSocketConnectionGroup.handshake.

Test/infra:
4. Rewrite the wait as real polling (getOrThrowUninterruptibly(20, SECONDS) in a try/catch over ~5 attempts, ~100 s total).
5. Separate the budgets: raise forkedProcessTimeoutInSeconds (e.g. 900) or keep the in-test wait well below the fork timeout.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingframeworkOpenICF-java-frameworktestsTest additions or fixes

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions