Repository navigation
Allow IS-07 and IS-12 WebSockets to share a port #525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||
| #include "nmos/control_protocol_ws_api.h" | ||||
|
|
||||
| #include <algorithm> | ||||
| #include <boost/algorithm/string/join.hpp> | ||||
| #include <boost/range/join.hpp> | ||||
| #include "cpprest/json_validator.h" | ||||
|
|
@@ -65,11 +66,13 @@ namespace nmos | |||
| if (!ws_validate_authorization(req, nmos::experimental::scopes::ncp)) { return false; } | ||||
| } | ||||
|
|
||||
| // For now just return true | ||||
| const auto& ws_ncp_path = req.request_uri().path(); | ||||
| slog::log<slog::severities::more_info>(gate, SLOG_FLF) << "Validating websocket connection to: " << ws_ncp_path; | ||||
|
|
||||
| return true; | ||||
| const auto is12_versions = nmos::is12_versions::from_settings(model.settings); | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs at least a read lock (and might as well use the same simple nmos-cpp/Development/nmos/node_api.cpp Line 41 in b6c8367
|
||||
| return std::any_of(is12_versions.begin(), is12_versions.end(), [&ws_ncp_path](const nmos::api_version& version) | ||||
| { | ||||
| return ws_ncp_path == U("/x-nmos/ncp/") + nmos::make_api_version(version); | ||||
| }); | ||||
| }; | ||||
| } | ||||
|
|
||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,8 +26,12 @@ namespace nmos | |
| std::vector<web::http::experimental::listener::http_listener> http_listeners; | ||
|
|
||
| // WebSocket APIs | ||
| std::map<host_port, std::pair<web::websockets::experimental::listener::websocket_listener_handlers, nmos::websockets>> ws_handlers; | ||
| std::vector<web::websockets::experimental::listener::websocket_listener> ws_listeners; | ||
| // each port may be shared by multiple APIs, each identified by a path prefix (e.g. "/x-nmos/events/") | ||
| // 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is because 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 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, thanks!
|
||
| std::map<host_port, web::websockets::experimental::listener::websocket_listener> ws_listeners; | ||
|
|
||
| // Server behaviours | ||
| std::vector<std::function<void()>> thread_functions; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make sense to capture
find_handlersvia ashared_ptrto avoid making four copies? not sure