fix issue leading to slowdown/unbounded memory growth in binary json processing#1200
Closed
tomqext wants to merge 63 commits intoRobotWebTools:ros2from
Conversation
* Autobahn WebSocket server * Bring back explicit Twisted dependency Still used directly by the UDP handler. * Warn when reactor wasn't running at shutdown hook * More descriptive websocket startup log msg * Remove extreneous OutgoingValve stop log * More comment for OutgoingValve * Fix --address parsing as int * Use Autobahn's ws url creation util method * Backwards compatibility for empty websocket addr This was the default in previous versions, but invalid input to Autobahn. * Factor out websocket onConnect We have access to this information in onOpen. * Add smoke test for rosbridge websocket server * Revert "Use Autobahn's ws url creation util method" This reverts commit 4224671. This create_url method doesn't exist yet in Ubuntu kinetic. * Preserve RosbridgeWebSocket Tornado ABI * Run rosbridge_websocket tests with ephemeral port Avoid network collisions and unnecessary configuration while testing. * Fix broken test create_url on kinetic * Smokier smoke test Send 100 large messages in each direction to saturate buffers.
The currently used proxy.get_topics function does not exists and results in the following error: `"AttributeError: 'module' object has no attribute 'get_topics'\n"` This change uses the existing `get_topics_and_types` method to get a list of topics.
…ge (RobotWebTools#443) autobahn.websocket.WebSocketServerProtocol.sendMessage asserts whether message is of type bytes, which fails for message of type bson.BSON
Under Python 3, values() returns a view-like object, and because that object is used outside the mutex, we were getting `RuntimeError: dictionary changed size during iteration` under some circumstances. This creates a copy of the values, restoring the Python 2 behaviour and fixing the problem.
The CBOR compression is already a huge win over JSON or PNG encoding, but it’s still suboptimal in some situations. This PR adds support for getting messages in their raw binary (ROS-serialized) format. This has benefits in the following cases: - Your application already knows how to parse messages in bag files (e.g. using [rosbag.js](https://github.com/cruise-automation/rosbag.js), which means that now you can use consistent code paths for both bags and live messages. - You want to parse messages as late as possible, or in parallel, e.g. only in the thread or WebWorker that cares about the message. Delaying the parsing of the message means that moving or copying the message to the thread is cheaper when its in binary form, since no serialization between threads is necessary. - You only care about part of the message, and don't need to parse the rest of it. - You really care about performance; no conversion between the ROS binary format and CBOR is done in the rosbridge_sever.
* Safer locking in PublisherConsistencyListener * Safer locking in ros_loader * Print QueueMessageHandler exceptions to stderr * Register before resuming outgoing valve * Don't pause a finished socket valve
* Expose autobahn externalPort parameter * Use ws port as default external port * Include websocket params in launchfile
When publishing a message to ROS (i.e. incoming from rosbridge_server's perspective), timestamps in the Header attributes all point to the same Time object iff the message contains multiple Header attributes (typically the case if a ROS message contains other ROS messages, e.g. ...Array-types) and rosparam use_sim_time is true.
* Run rosbridge_server websocket tests serial Running these tests in parallel may make them less reliable. * Reduce websocket smoke test load Appears to be failing intermittently due to a client issue. Co-authored-by: Jihoon Lee <jihoonlee.in@gmail.com>
* rosbridge_library, rosbridge_server: Update package format Add Python3 conditional dependencies where applicable. * rosbridge_library: Fix pngcompression for Python 3 * rosapi: Use catkin_install_python for scripts
…bTools#469) For Kinetic 0.10.3 version don't add the allowNullOrigin option. For everything else add it and default to True because that is consistent with the original behaviour of this package. Add websocket_null_origin to launch args
…botWebTools#470) fixes bug introduced in RobotWebTools#468
* Unblock QueueMessageHandler.handle_message The thread was holding the lock while pushing to a potentially blocking function. Rewrite the logic and use a deque while we're at it. * Add test for subscription queue behavior Guarantee that the queue drops messages when blocked.
* Lock access to SubscriberManager public methods Prevent subscribe during unsubscribe critical section. * Unsubscribing an unsubscribed topic is an error This branch must not be ignored. * Cleanup some redundant syntax in subscribers impl
* Add script for dockerized development shell * Fix queue dropping test
* Don't block Subscription.unregister() * Don't add messages to finished queue handler * Decouple incoming WS handling from server thread
Service calls with non empty requests (e.g. /rosapi/topics_for_type) were crashing due to filter's return type in python 3.
max_message_size has to be integer when initializating using tornado.
…y concatenate str (not bytes) to str (RobotWebTools#514) Co-authored-by: Dmitri <dmitri@dmitri.com>
As of python3 sockets will input and output byte strings instead of String objects. This commits does the necessary coversions inside the tcp_handler, but it might not be the most thoughtful solution but just what I did to make it work with python3.
This is required to work with python3 socket library
Fixes Python 2 unicode issues. Supersedes RobotWebTools#539 to fix regression in RobotWebTools#514
…ch is required when using python3. Fixes RobotWebTools#527. (RobotWebTools#540)
* fix python-future dependency This was not fixed with fadffa0 The dependency was added with RobotWebTools#541 Python doesn't have build dependencies, only exec dependencies * update ros-tooling/action-ros-ci See: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/ This was fixed upstream - ros-tooling/action-ros-ci#429 - ros-tooling/action-ros-ci#430 * update ros-tooling/setup-ros See: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/ This is needed for upstream fix - ros-tooling/setup-ros#303
This allows update from binary deb package over 0.11.x. - bump version to 0.11.130 (changes over 0.11.13) - add modifications information to description - compatible with 0.11.x - without future bug fixes (0.11.14 and later) - update changelog Related to #1
- explains binary packaging - installation - switching back to original - and future updates policy Related to #1
This reverts commit b9d793c.
This reverts commit 2d21f98.
- support namespaced parameters for rosbridge
tomqext
deleted the
ERD-1923_ros1_fix_decode_binary_websocket_frames_before_json_parse_internal
branch
Author
|
sorry, this one was supposed to be internal 😅 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See upstream PR for more info #1199