[#112] Give each Jetty connection its own endpoint and close cached principals

[vharseko]

Part 2 of #112: the Jetty principal lifecycle gaps.

Problem

SinglePrincipal doubled as the cached principal (one per principal name, cached forever by OpenICFWebSocketCreator.principalCache) and the per-connection Jetty endpoint:

• hasCloseBeenCalled was set on the first onWebSocketClose and never reset — every later connection of the same cached principal had its close event silently swallowed (OperationMessageListener.onClose never invoked).
• WebSocketConnectionHolder.close() was never called at all on the Jetty server, so the WebSocketConnectionGroup was never told a holder went away; every reconnect appended another duplicate holder entry.
• session and context were single fields shared by all connections of a name — two concurrent connections of one principal clobbered each other.
• OpenICFWebSocketServletBase.destroy() never closed cached principals (doClose() never ran, close listeners never fired), never released a lazily acquired ConnectorFramework, and the lazily created scheduler was never flagged private, so destroy() never shut it down either.

Fix

Follows the shape of the Grizzly server (per-connection endpoint object, principals closed from the application-level close()). The holder-drop-on-close part is new — Grizzly has the same gap on both its server and client endpoints, tracked separately in #118:

• New per-connection endpoint OpenICFWebSocket: own session (volatile), context, AtomicBoolean close guard and single send thread (shut down when the connection closes). onWebSocketClose now also calls adapter.close(), so the connection group finally drops the departed holder.
• SinglePrincipal shrinks to the pure principal role (254 → 75 lines).
• OpenICFWebSocketCreator hands out a fresh endpoint per connection and gains close(): principalIsShuttingDown for every cached principal across all groups (one pass over the groups), principal.close() for every cached principal, cache cleared, health-check task cancelled. After close() the creator rejects new connections (volatile closed flag: createWebSocket() responds 503, authenticate() returns null).
• OpenICFWebSocketServletBase.destroy() invokes creator.close() before super.destroy(), releases the lazily acquired framework (privateConnectorFramework is now set on the lazy path) and shuts the lazily created scheduler down.

Tests

• ReconnectSendTest reworked for the per-connection endpoint: send works after reconnect, the close of a later connection is delivered, duplicate close events of one connection stay suppressed.
• ReconnectSendTest.testGroupDropsHolderOnClose: drives a real WebSocketConnectionGroup handshake and asserts the group drops the departed holder after onWebSocketClose — fails without the adapter.close() call.
• New PrincipalLifecycleTest: principal cached per name, distinct endpoint per connection, close() ends the lifecycle of cached principals and empties the cache.
• New ServletLifecycleTest: destroy() closes the creator (principals' close listeners fire, new connections rejected), shuts the private scheduler down and releases the lazily acquired framework.

Follow-ups: #118 (Grizzly endpoints never call adapter.close()), #119 (migrate the remaining Jetty classes off the deprecated org.eclipse.jetty.util.log.Log).

Verified: connector-server-jetty 34/34.

[maximthomas]

A few things before merge.

1. Grizzly does not already get this right

Mirrors the Grizzly server, which already gets this right

It doesn't. OpenICFWebSocketApplication.OpenICFWebSocket.onClose(DataFrame) (:256) drains its listeners but never calls adapter.close() either — same for the client endpoint in ClientRemoteConnectorInfoManager (:617). Since OpenICFServerAdapter.onClose (:102) only logs, the closeListener registered at WebSocketConnectionGroup:116 fires on no endpoint today. This PR makes Jetty the first.

Not a blocker, but please fix the description and open a follow-up — otherwise the servers just diverge in the opposite direction.

2. privateConnectorFramework has the identical bug, one line away

The PR fixes the executor leak but leaves its twin in the method right above:

protected ConnectorFramework getConnectorFramework() {
    if (null == connectorFramework) {
        ConnectorFrameworkFactory cf = getConnectorFrameworkFactory();
        if (null != cf) {
            connectorFramework = cf.acquire();   // <-- privateConnectorFramework never set

So destroy() never releases a lazily acquired framework. The test servlet (OpenICFWebSocketServlet, no-arg ctor) takes exactly this path. Same one-liner as the scheduler fix.

3. The headline bug isn't tested

The stated defect is "every reconnect appended another duplicate holder entry." Neither new test touches WebSocketConnectionGroup — ReconnectSendTest only counts OperationMessageListener.onClose calls, which was the other bug. A test that drives a handshake and asserts the group's holder list shrinks after onWebSocketClose would cover the actual fix. Also nothing covers destroy() (creator closed + private scheduler shut down), which is two of the four bullets.

---

Cheap hardening

session should be volatile. context got volatile plus a comment about cross-thread reads; session didn't, yet getSession() is read from the send-executor thread:

private Session session;                              // written on the Jetty connect thread
...
return sendExecutor.submit(() -> {
    getSession().getRemote().sendBytes(...);          // read on the send thread
});

The happens-before exists today only via the framework's internals, not this class. One keyword.

closed should be AtomicBoolean. It's the invariant the new tests are built on, but they only exercise same-thread duplicates:

if (!closed.compareAndSet(false, true)) {
    return;
}

tryClose() fires a close handshake from inside close handling (onWebSocketClose → adapter.close() → tryClose()). Jetty tolerates it, but:

if (null != current && current.isOpen()) {
    current.close(StatusCode.NORMAL, "Shutdown");
}

OpenICFWebSocketCreator stays fully usable after close() — principalCache.clear() runs, then the next authenticate() mints a fresh SinglePrincipal bound to a framework destroy() is about to release. A volatile boolean closed checked in createWebSocket() closes the window.

Nits

• ReconnectSendTest.SENT is static final and never reset — fine with one method, a landmine at two. Make it an instance field.
• destroy(): run creator.close() before super.destroy(), and move websocketCreator = null into a finally.
• creator.close() nests principals × groups (Grizzly nests groups × principal); the inner globalConnectionGroups.remove(...) means later principals never see already-removed groups. One pass over groups is clearer.
• authenticate() still does get/putIfAbsent/get — the losing thread's SinglePrincipal is dropped without close(). computeIfAbsent fixes it.
• Utils.newThreadFactory(null, "...", true) — every other call site in the repo passes false. Intentional (daemon threads for per-connection senders seems right), or a slip?
• onWebSocketError logs at ok/trace, so errors are invisible in production. Not a regression (was debug), but warn would earn its keep.
• sendBytes/sendString are near-identical 18-line blocks; a shared helper halves them.
• Three copyright spellings in one PR: Portions Copyrighted 2026 3A Systems, LLC. / Portions copyright 2025-2026 3A Systems LLC. / Portions Copyrighted 2026 3A Systems, LLC.
• New class uses org.identityconnectors.common.logging.Log while its neighbours use the deprecated org.eclipse.jetty.util.log.Log. The new choice is right — worth migrating the other two separately.

[vharseko]

Thanks for the thorough review — everything addressed in 1039419, follow-ups filed.

1. Grizzly — confirmed, you're right: neither the Grizzly server endpoint (OpenICFWebSocketApplication.OpenICFWebSocket.onClose) nor the client endpoint in ClientRemoteConnectorInfoManager ever calls adapter.close(), so the group's closeListener never fires there either. Reworded the PR description and filed #118.

2. privateConnectorFramework — fixed: getConnectorFramework() now sets the flag on the lazy path, and the new ServletLifecycleTest asserts the lazily acquired framework is actually released by destroy() (the test fails without the one-liner).

3. Tests for the headline bug — added ReconnectSendTest.testGroupDropsHolderOnClose: it drives a real WebSocketConnectionGroup handshake and asserts the group drops the departed holder after onWebSocketClose, including a reconnect cycle. Verified it fails with the adapter.close() call removed. destroy() is covered by ServletLifecycleTest: creator closed (principals' close listeners fire, new connections rejected), private scheduler shut down, framework released.

Hardening — all applied: session is volatile, closed is an AtomicBoolean CAS, tryClose() checks isOpen(), and the creator gets a volatile closed flag — after close(), createWebSocket() responds 503 and authenticate() returns null.

Nits

• SENT → per-instance counter.
• destroy() reordered: creator.close() before super.destroy(), field cleared in finally.
• creator.close() now walks the groups once. Good catch — beyond clarity, the old nesting skipped principalIsShuttingDown for later principals on already-removed groups, so their pending requests were never cancelled.
• authenticate() → computeIfAbsent.
• Daemon flag: intentional — per-connection send threads must not pin JVM shutdown, and the executor is also shut down explicitly on close now. One small correction: ConnectionManager (the client scheduler) passes true as well, so it isn't the only daemon call site.
• onWebSocketError → warn.
• sendBytes/sendString → shared submitSend() helper.
• Copyright: the new file now carries the canonical Portions Copyrighted 2026 3A Systems, LLC; the Portions copyright 2025-2026 3A Systems LLC. lines are pre-existing headers that only had their end year extended, so they were left as-is.
• Logger migration for the two remaining classes: filed connector-server-jetty: migrate off deprecated org.eclipse.jetty.util.log.Log #119.

connector-server-jetty: 34/34. The branch still carries #110's commits (it was squash-merged); will rebase onto master before merge.