Repository navigation
Allow IS-07 and IS-12 WebSockets to share a port - #525
iibaranov-IG wants to merge 2 commits into
Conversation
… used by Registry and Node. Enhance WebSocket handling to support port sharing and future extensions.
|
Hi @iibaranov-IG , thank you so much for your contribution. I have refined your suggestions to make them more general and future-proof. |
| // in order to maintain a separate set of connections (nmos::websockets) per API, even when a port is shared | ||
| typedef std::map<utility::string_t, std::pair<web::websockets::experimental::listener::websocket_listener_handlers, nmos::websockets>> ws_api_handlers; | ||
| std::map<host_port, ws_api_handlers> ws_handlers; | ||
| std::deque<web::websockets::experimental::listener::websocket_listener_handlers> combined_ws_handlers; // for APIs sharing a port, the handlers are combined into one set of handlers |
There was a problem hiding this comment.
This is because make_ws_api_listener() captures the handlers by reference; each websocket_listener ends up storing a std::reference_wrapper pointing directly at the memory address of a specific websocket_listener_handlers object living inside combined_ws_handlers.
If combined_ws_handlers were a std::vector, calling .push_back() in a loop (once per WebSocket port being configured) can trigger reallocation when the vector's capacity is exceeded. Reallocation copies/moves all existing elements to a new, larger memory block and frees the old one. That invalidates every reference/pointer/iterator to previously inserted elements; see example below:
e.g. if with a std::vector
for (auto& ws_handler : node_server.ws_handlers) // e.g. 2 iterations: events port, control protocol port
{
...
node_server.combined_ws_handlers.push_back(nmos::make_ws_api_router(path_handlers));
// if this push_back reallocates, any websocket_listener already
// constructed in a PREVIOUS iteration (which holds std::ref to an
// earlier combined_ws_handlers element) is now dangling!
node_server.ws_listeners.insert({ ws_handler.first, nmos::make_ws_api_listener(..., node_server.combined_ws_handlers.back(), ...) });
}
But std::deque guarantees that inserting at the front or back never invalidates references or pointers to other elements (only iterators may be invalidated, but we're not holding onto iterators - we're holding onto references inside std::ref, which deque preserves across push_back). Internally, deque is implemented as a sequence of fixed-size chunks, so growing it just adds a new chunk rather than reallocating/moving all existing elements.
This matches exactly what we need here: every websocket_listener_handlers added in earlier loop iterations must remain at a stable address for the entire lifetime of node_server, even as more are added in later iterations.
Fixes #422
When
events_ws_portandcontrol_protocol_ws_portmatched,make_node_serverrejected the configuration before creating its listeners. The WebSocket endpoints are already route-distinct: IS-07 uses/x-nmos/events/...and IS-12 uses/x-nmos/ncp/v1.0.This change installs one listener for a shared port and dispatches handshake, open, close, and message callbacks by endpoint path. IS-07 and IS-12 retain separate subscription/connection state and message threads. Existing configurations with distinct ports continue to create independent listeners. IS-12 validation now accepts only configured
/x-nmos/ncp/<version>routes, so invalid or conflicting paths are rejected instead of being claimed by the control endpoint.Tests cover same-port, separate-port, IS-07-only, and valid/invalid/conflicting IS-12 routes.
Validation: Debug build and all 189 CTest tests pass on macOS/Apple Silicon.