Follow-up to the review discussion in #108 (see also #107).
Problem
OpenICFServerAdapter.onMessage hands every incoming WebSocket message to a shared unbounded cached thread pool (ConnectorFramework.executeMessage, ConnectorFramework.java:83). Two messages delivered in order on the same socket can therefore be processed out of order.
#108 fixed the most visible instance — an operation response overtaking the handshake response on a freshly opened socket, which silently dropped the response and hung testAsynchronousBatch for 9 minutes — by briefly waiting for the in-flight handshake (awaitHandshake, 500 ms poll). But that only synchronizes against the handshake boundary:
- two operation responses for the same request can still be reordered (e.g. batch results — currently compensated by the
responseQueue re-sorting in BatchApiOpImpl);
- any future message type that relies on arrival order has to add its own compensation.
Proposal
Serialize dispatch per socket: keep the shared pool for cross-socket concurrency, but ensure messages of one socket are processed in arrival order — e.g. a per-socket serial task queue draining onto the pool (one in-flight task per socket at a time).
This would:
- eliminate the whole class of reordering races instead of patching instances;
- make
awaitHandshake in OpenICFServerAdapter unnecessary;
- allow removing the per-message ordering compensation elsewhere.
Notes
- Bounding per-socket processing also removes the risk of many pool threads parked in
awaitHandshake on sockets that never complete a handshake.
- Care needed with in-message blocking (a serial queue must not deadlock on messages whose processing waits for another message of the same socket — the handshake wait would be gone, which is the main such case today).
Follow-up to the review discussion in #108 (see also #107).
Problem
OpenICFServerAdapter.onMessagehands every incoming WebSocket message to a shared unbounded cached thread pool (ConnectorFramework.executeMessage,ConnectorFramework.java:83). Two messages delivered in order on the same socket can therefore be processed out of order.#108 fixed the most visible instance — an operation response overtaking the handshake response on a freshly opened socket, which silently dropped the response and hung
testAsynchronousBatchfor 9 minutes — by briefly waiting for the in-flight handshake (awaitHandshake, 500 ms poll). But that only synchronizes against the handshake boundary:responseQueuere-sorting inBatchApiOpImpl);Proposal
Serialize dispatch per socket: keep the shared pool for cross-socket concurrency, but ensure messages of one socket are processed in arrival order — e.g. a per-socket serial task queue draining onto the pool (one in-flight task per socket at a time).
This would:
awaitHandshakeinOpenICFServerAdapterunnecessary;Notes
awaitHandshakeon sockets that never complete a handshake.