changes between 1.12.7 and 1.13.4 for backporting #1205
Conversation
* refactor test_rosbag_storage * fix rosbag::View::iterator copy assignment operator the compiler-generated copy assignment operator did lead to segfault and memory leaks.
…other thread (#1013) * Avoid deleting XmlRpcClient's while they are being still in use on another thread * Acquire clients_mutex_ before deleting the clients * Remove the timed wait for the clients to become not in use * Only delete and erase clients that are not in use * Clients that would still be in use would delete themselves on release * Wait for clients that are in use to finish in XmlRpcManager::shutdown
In a multimaster environment where a topic has multiple publishers, when a node drops out abruptly (host is shutdown), a single subscriber update on that topic will cause multiple threads to be created (one for each host) in order to resolve the topic location. This cause a thread leak as host which are turned off will not respond and when they come back online, the xmlrpc URI is changed causing a connection refused error at the socket layer. This fix catches the connection refused error and terminate the thread with the understanding that if the connection is refused, the rosnode cannot be reached now or never. This effectively prevents thread leak. Note: if the remote host where the rosnode is thought to be never comes back up, then the thread will still be leaked as the exception received is a host unreachable type. This is intentional to avoid abruptly terminating the thread in case of a temporary DNS failure.
* Fix bug in transport_tcp It assumes that the `connect` method of non-blocking scoket should return -1 and `last_socket_error()` should return `ROS_SOCKETS_ASYNCHRONOUS_CONNECT_RETURN`(=`EINPROGRESS`). But a non-blocking `connect` can return 0 when TCP connection to 127.0.0.1 (localhost). [http://stackoverflow.com/questions/14027326/can-connect-return-0-with-non-blocing-socket](http://stackoverflow.com/questions/14027326/can-connect-return-0-with-non-blocing-socket) * Modify code format Modify code format
* Fix race condition that lead to miss first message Callback queue waits for callback from "callOne" method. When internal queue is not empty this last method succeeded even if id info mapping does not contains related callback's id. In this case, first callback (for one id) is never called since "addCallback" method first push callback into internal queue and *then* set info mapping. So id info mapping has to be set before push callback into internal queue. Otherwise first message could be ignored. * Added test for addCallback race condition
* Add option to reset timer when time moved backwards * refactor logic
* Added logging output when `roslogging` cannot change permissions Added better differentiated logging output to `roslogging` so that problems with permission is made clear to the user. Accompanying test have also been added. * Removed testing, updated warning message and fixed formatting Removed testing since test folder should not be stored together with tests. Since testing group permission requires intervention outside the test harness the test it self is also removed. Updated the warning message to include `WARNING` and updated to using `%` formatting.
* Check /clock publication neatly in publishtest - Use time.sleep because rospy.sleep(0.1) hangs if /clock is not published - Add timeout for clock publication * Add comment explaining use of time.sleep. * Use logwarn_throttle not to flood the console
…o direct control flow hijacking (#1092) * A fix to a critical stack buffer overflow vulnerability which leads to control flow hi-jacking. * Much more simple fix for the stack overflow bug
…d a swap function instead (#1000) * Made copying rosbag::Bag a compiler error to prevent crashes * Added Bag::swap(Bag&) and rosbag::swap(Bag&, Bag&) * Fixed bugs in Bag::swap * Added tests for Bag::swap
After an update of gcc and glibc roscpp started to fail builds with the error:
/home/rojkov/work/ros/build/tmp-glibc/work/i586-oe-linux/roscpp/1.11.21-r0/ros_comm-1.11.21/clients/roscpp/src/libros/transport/transport_udp.cpp:579:25: error: 'writev' was not declared in this scope
ssize_t num_bytes = writev(sock_, iov, 2);
^~~~~~
According to POSIX.1-2001 the function writev() is declared in sys/uio.h.
The patch includes the missing header for POSIX compliant systems.
* add SteadyTimer based on SteadyTime (which uses the CLOCK_MONOTONIC). This timer is not influenced by time jumps of the system time, so ideal for things like periodic checks of timeout/heartbeat, etc... * fix timer_manager to really use a steady clock when needed This is a bit of a hack, since up to boost version 1.61 the time of the steady clock is always converted to system clock, which is then used for the wait... which obviously doesn't help if we explicitly want the steady clock. So as a workaround, include the backported versions of the boost condition variable if boost version is not recent enough. * add tests for SteadyTimer * [test] add -pthread to make some tests compile not sure if this is only need in my case on ROS indigo... * use SteadyTime for some timeouts * add some checks to make sure the backported boost headers are included if needed * specialize TimerManager threadFunc for SteadyTimer to avoid the typeid check and make really sure the correct boost condition wait_until implementation is used * Revert "[test] add -pthread to make some tests compile" This reverts commit f62a3f2. * set minimum version for rostime * mostly spaces
* Add close_half_closed_sockets function * Call close_half_closed_sockets in xmlrpcapi by default
* Don't try to set unknown socket options These are not avaible on FreeBSD, for example * individualize ifdefs * fix whitespace
…() (#1191) - Only triggered if reduce_overlap_ = true - When iters_.size() == 1 and iters_.pop_back() gets called in the loop, the next loop condition check would read from iters_.back(), but iters_ would be empty by then.
* Test bzfile_ before reading/writing/closing * Test lz4stream before reading/writing
* More agile demux. Publishers in demux are no longer destroyed and recreated when switching, which results in much faster switching behavior. The previous version took even 10 seconds to start publishing on the newly selected topic (all on localhost). Please, comment if you feel the default behavior should stay as the old was, and this new behavior should be triggered by a parameter. * update style
|
CI fails since the changes require a new release of |
|
@ros/ros_team @ros/ros_comm-maintainers Waiting for your feedback... |
|
I did a brief once over on all of the proposed code. Nothing in particular stuck out at me as very problematic, or API/ABI breaking. I also looked at the question marked ones in more detail:
I took another look at commit 64920f2, and it seems OK to me. I agree with you that there is some scope for regression, but overall I think it is worthwhile.
This one is harder to quantify, since it essentially comes down to the behavior of
This one I'm concerned that people are relying on the current behavior without realizing it. While it is clearly incorrect, the next update they do to their Kinetic packages will break their robot in ways they don't understand. I'd leave this one out. |
|
@clalancette Thank you for the review. I agree that "using poll() in favor of select() in the XmlRPCDispatcher #1133" has a good potential of introducing regressions. Since it has been tested by a few parties for the last few month though and they would really like to see this fixed in an LTS release I am leaning towards taking the chances 😐 While #1104 also provides some improvements it won't solve all the problematic cases so the more invasive change is still necessary for some use cases. I see your point on "[bug] fixes 1158 #1159". The problem is that without the fix other valid use cases are not possible (passing a parameter only to the first node but not to the second). Therefore I prefer to keep this for unblocking valid use cases over potentially breaking users relying on wrong API behavior. |
dirk-thomas
changed the title
* catch exception with `socket.TCP_INFO` on WSL fixes issue #1207 this only catches socket error 92 * Update util.py * Update util.py * avoid unnecessary changes, change import order
|
@ros-pull-request-builder retest this please |
|
@ros-pull-request-builder retest this please |
The following list of changes has been integrated into ros_comm 1.13.4 (Lunar) since the last Kinetic release (1.12.7).
Backported: (these changes are part of this PR)
fix open mode on Windows fix open mode on Windows #1005
fix BZip2 inclusion Fix BZip2 inclusion #1016
make roslaunch-check respect if/unless attribute on include Make roslaunch-check respect if/unless attribute on <include> #998
fix rosbag::View::iterator copy assignment operator fix rosbag::View::iterator copy assignment operator #1017
fix rosmsg show from bag fix rosmsg show from bag #1006
add subscriber to connection log messages Add subscriber to connection log messages. #1023
ensure cwd exists ensure cwd exists #1031
Sleep in rospy wait_for_service even if exceptions raised Sleep in rospy wait_for_service even if exceptions raised #1025
Avoid deleting XmlRpcClient's while they are being still in use on another thread Avoid deleting XmlRpcClient's while they are being still in use on another thread #1013
abort topic lookup on connection refused Abort topic lookup on connection refused #1044
fix bug in transport_tcp Fix bug in transport_tcp #1050
fix race condition that lead to miss first message Fix race condition that lead to miss first message (lunar) #1058
ensure pid file is removed on exit roslaunch: ensure pid file is removed on exit #1057
changed the check command output to be a little bit more specific Changed the check command output to be a little bit more specific. #1067
fix pid file removing condition [roslaunch] Fix pid file removing condition #1084
add option to reset timer when time moved backwards [rospy] Add option to reset timer when time moved backwards #1083
add missing mutex lock for publisher links add missing mutex lock for publisher links #1090
improve rospy.logXXX_throttle performance [rospy] Improve rospy.logXXX_throttle performance #1091
added logging output when
rosloggingcannot change permissions Added logging output whenrosloggingcannot change permissions #1068Check /clock publication neatly in publishtest [rostest] Check /clock publication neatly in publishtest #973
fix to a critical stack buffer overflow vulnerability A fix to a critical stack buffer overflow vulnerability which leads to direct control flow hijacking #1092
only launch core nodes if master was launched by roslaunch only launch core nodes if master was launched by roslaunch #1098
made copying rosbag::Bag a compiler error to prevent crashes and added a swap function instead Made copying rosbag::Bag a compiler error to prevent crashes and added a swap function instead #1000
add missing header for writev [roscpp] add missing header for writev(). #1105
add SteadyTimer Add SteadyTimer #1014
close CLOSE_WAIT sockets by default Close CLOSE_WAIT sockets by default #1104
fix handling connections without indices fix handling connections without indices #1109
fix rostopic prining long integers fix rostopic prining long integers #1110
update tests to match stringify changes update tests to match stringify changes #1125
ignore headers with zero stamp in statistics ignore headers with zero stamp in statistics #1127
improves the stability of SteadyTimerHelper improve the stability of SteadyTimerHelper #1128
Improve steady timer tests Improve steady timer tests #1132
xmlrpc_manager: use SteadyTime for timeout xmlrpc_manager: use SteadyTime for timeout #1134
removed deprecated dynamic exception specifications Removed deprecated dynamic exception specifications #1137
only use CLOCK_MONOTONIC if not on osx only use CLOCK_MONOTONIC if not on osx #1142
Improved whitespace to fix g++ 7 warning Improved whitespace to fix g++ 7 warning #1138
using poll() in favor of select() in the XmlRPCDispatcher Using poll() in favor of select() in the XmlRPCDispatcher (extended) #1133
fix rostopic hz and bw in Python 3 Fix rostopic hz and bw in Python 3 #1126
don't direct users to build rosout with rosmake Don't direct users to build rosout with rosmake. #1140
use not deprecated console_bridge macros and undefine the deprecated ones use not deprecated console_bridge macros and undefine the deprecated ones #1149
fix rosbag API for Python 3 Fix rosbag API for Python 3 #1150
sort the output of rosnode info Sort output of rosnode info #1160
Minor fixes for compatibility with both Python 2 and 3
rosnode: Minor fixes for compatibility with both Python 2 & 3 #1166
[bug] fixes 1158 [bug] fixes #1158 #1159
rosconsole: replaced 'while(0)' by 'while(false)' rosconsole: replaced 'while(0)' by 'while(false)' in ros/assert.h and ros/console.h #1179
change rospy.Rate hz type from int to float Change rospy.Rate hz type description from int to float #1177
don't try to set unknown socket options Don't try to set unknown socket options #1172
rosnode: Return exit code 1 if there is an error rosnode: Return exit code 1 if there is an error. #1178
fixed an out of bounds read in void rosbag::View::iterator::increment() Fixed an out of bounds read in void rosbag::View::iterator::increment() #1191
test bzfile_ and lz4s_ before reading/writing/closing Test bzfile_ and lz4s_ before reading/writing/closing #1183
more agile demux More agile demux. #1196
catch exception with
socket.TCP_INFOon WSL catch exception withsocket.TCP_INFOon WSL #1212fix roswtf tests fix roswtf tests #1102
Not backported:
add node name to the output in rosout.log Add node name to the output in rosout.log #918
deprecate old spinner behavior deprecate old spinner behavior #900
proper logrotation for rosout [rosout] Proper logrotation for rosout #991
fix test generation Fix test generation #1009
expose rate-control-topic and rate-control-max-delay args to command line tool expose rate-control-topic and rate-control-max-delay args to command … #1015
force a write of all user-space buffered data for stdout in Formatter::print Force a write of all user-space buffered data for stdout in Formatter::print #1012
fix migration rule Fix migration rule #1011
Removed "ROS time moved backwards" log message Removed "ROS time moved backwards" log message #1027
rosconsole: remove extra semicolon in definition of macro ROSCONSOLE_PRINTF_ATTRIBUTE(a, b) rosconsole: remove extra semicolon in definition of macro ROSCONSOLE_PRINTF_ATTRIBUTE(a, b) #1056
add XXH_NAMESPACE, for namespace emulation in C Add XXH_NAMESPACE, for namespace emulation in C #1065
add rospy.logXXX_once [rospy] Add rospy.logXXX_once #1041
write log for class method with class name for rospy [rospy] Write log for class method with class name for rospy #1043
add yaml type for param tag [roslaunch] Add yaml type for <param> tag #1045
add named loggers to rospy Add named loggers to rospy #948
improve roswtf tests improve roswtf tests #1101
moved code from init to initializer [rosout] Moved code from init to initializer #990
now allowing python_logging.yaml for logging configuration [rosgraph] now allowing python_logging.yaml for logging configuration #1061
switch to libb64 for base64 encoding/decoding Switch to libb64 for base64 encoding/decoding. #1046
clean the namespace to get rid of double or trailing forward slashes Clean the namespace to get rid of double or trailing forward slashes … #1100
Add $(dirname) to get directory of current launch file Add $(dirname) to get directory of current launch file. #1103
make RospyLogger.findCaller compatible with Python3 Make RospyLogger.findCaller compatible with Python3 (also, unbreak ROS on Python3) #1121
improves lunar-devel jenkins test stability Improves lunar-devel jenkins test stability #1135
fixed g++ 7 fall through warnings Fixed g++ 7 fall through warnings #1139
fix stack frame identification in rospy logging Fix stack frame identification in rospy logging. #1141
don't need to see the full stack trace for ResourceNotFound Don't need to see the full stack trace for ResourceNotFound #1147
improved rospy core logger test improved rospy core logger test. #1144
fix publishing of selected topics from bag file, regression from d1d1711 Fix publishing of selected topics from bag file #1156
enable service unavailable log in rospy wait_for_service Enable service unavailable log in rospy wait_for_service #1026
use python constants rather than hardcoded integers for error codes use python defined error codes rather than hardcoded integers #1174
one more hardcoded python error code one more hardcoded python error code #1181
add --set-master-logger-level option for 'rosmaster' to output LOG_API add --set-master-logger-level option for 'rosmaster' to output LOG_API #1180
enabled rosbag::Bag move operations if compiler support is available Enabled rosbag::Bag move operations if compiler support is available #1189
added rosbag::Bag::isOpen Added rosbag::Bag::isOpen #1190
static-cast unused parameters in TransportTCP Static-cast unused parameters in TransportTCP. #1195
add stealth mode for topic_tools/relay add stealth mode for topic_tools/relay #1155
fix CMake Error: xmlrpcvalue_base64 not built by Fix CMake Error: xmlrpcvalue_base64 not built by.. #1197
fixed corrupted messages when reopening a rosbag with a different file Fixed corrupted messages when reopening a rosbag with a different file (#1176) #1192
TOPIC TOOLS: add initial_topic param TOPIC TOOLS: add initial_topic param #1199
reenable topic_tools tests reenable topic_tools tests #1200
fix uri in message fix uri in message #1213
replaced deprecated lz4 function call Replaced deprecated lz4 function call #1136
@ros/ros_team @ros/ros_comm-maintainers Please comment on the decision which changes to (not) backport. Especially look at the changes marked with a "?".