Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Fix some errors in some probably not frequented code paths#1410

Closed
awesomebytes wants to merge 98 commits intoros:kinetic-develfrom
awesomebytes:fix_unlikely_to_happen_errors
Closed

Fix some errors in some probably not frequented code paths#1410
awesomebytes wants to merge 98 commits intoros:kinetic-develfrom
awesomebytes:fix_unlikely_to_happen_errors

Conversation

@awesomebytes
Copy link
Copy Markdown
Contributor

These were detected while cythonizing recursively some files.

Cython (via cythonize) tries to create equivalent .c source code files and it acts kind of like a static code analyser.

dirk-thomas and others added 30 commits October 25, 2017 13:46
* 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 (ros#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
…#1068)

* 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 (ros#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 (ros#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
dirk-thomas and others added 20 commits February 9, 2018 12:06
I added some sys.exit(1)'s to rosbag commands to help with scripting.
Specifically, I expected a call to `rosbag info test.bag.active` to
return an error when the bag needed re-indexing.
* Performance improvement for lower/upper bound

Changed updateQueries() to use member functions of `multiset` instead of `std::lower_bound` and `std::upper_bound`

* Fixed warnings for C++98

Changed the calls to lower/upper bounds so they dont use the "new" C++11 extended initializer lists.
Switching to using epoll system calls to improve performance of the topic polling code by a factor of 2. This required disabling the addDelMultiThread test.
ROS_IPV6 environment variable was checked after first connection could be established.
This commit moves this check into early initialization.
…#1313)

* adding decompress to free(state) before return

* copy paste typo error on roslz4_decompressEnd

* revert unnecessary change
Previously, this was causing `TypeError: 'module' object is not
callable`, which was not noticed due to the bare `except:` in the test.
* Add TCP_INFO availability check

* simplify diff / patch
* Add add_rostest_gmock function

* Increase catkin required version due to new GMock API
Address a long standing issue where the `sockets_changed_` flag would never be reset causing the native poll set to be re-created everytime doing a poll.
When a process on a remote machine dies, the `process_died` callback of
the server is called.  However, the process listeners given to
`ROSLaunchParent` need to be forwarded to the server, otherwise they
cannot be called.
* fix xmlrpc timeout using monotonic clock

in XmlRpcDispatch::work the _endTime check was using system time instead of monotonic time,
so when time was set back it didn't process all events..

* replace clock_gettime for OSX

* xmlrpcpp: copy code from ros_steadytime for getTime

* use ros_steadytime

which depends on ros/roscpp_core#73
and assumes that it will be available with rostime 0.6.9

* fold build and run depend into depend tag to avoid version to be repeated

* add missing namespace to ros_steadytime

* add missing target_link_libraries

* add missing lib to tests
changes between 1.12.12 and 1.13.6 for backporting
These were detected while cythonizing recursively some files
@mikepurvis
Copy link
Copy Markdown
Member

Nice work, these all look like legit fixes to me! 👍

@dirk-thomas dirk-thomas changed the base branch from kinetic-devel to melodic-devel May 29, 2018 18:10
@dirk-thomas
Copy link
Copy Markdown
Member

I just changed the target branch to melodic-devel. Please rebase the patch to apply cleanly to that branch. Thanks.

@dirk-thomas dirk-thomas changed the base branch from melodic-devel to kinetic-devel May 30, 2018 16:20
@dirk-thomas dirk-thomas changed the base branch from kinetic-devel to melodic-devel May 30, 2018 16:21
@awesomebytes awesomebytes changed the base branch from melodic-devel to kinetic-devel June 1, 2018 07:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.