Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix a bug that using a destroyed connection object. #1950

Merged
merged 1 commit into from
May 21, 2020
Merged

Fix a bug that using a destroyed connection object. #1950

merged 1 commit into from
May 21, 2020

Conversation

Barry-Xu-2018
Copy link
Contributor

there is a case that accessing a connection object which was destroyed by
ConnectionManager and TransportSubscriberLink/TransportPublicationLink
while getting a close event in pollset to call Connection::drop.

Signed-off-by: Barry Xu Barry.Xu@sony.com

@Barry-Xu-2018
Copy link
Contributor Author

This fixing is related to #1949.

@Barry-Xu-2018 Barry-Xu-2018 mentioned this pull request May 11, 2020
Closed
fujitatomoya
fujitatomoya approved these changes May 11, 2020
View reviewed changes
Copy link
Contributor

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but requesting others to review.

@fujitatomoya
Copy link
Contributor

@dirk-thomas @mjcarroll

would you take a look at this?

mjcarroll
mjcarroll approved these changes May 14, 2020
View reviewed changes
Copy link
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me. Thanks for the contribution.

@dirk-thomas
Copy link
Member

This change seems to be closely related to #1940. It doesn't seem that both patches should be applied. Please clarify.

@Barry-Xu-2018 Barry-Xu-2018 mentioned this pull request May 15, 2020
Merged
@dirk-thomas
Copy link
Member

With #1940 merged this needs to be rebased. Please still clarify why this is (still) necessary.

@Barry-Xu-2018
Fix a bug that using a destroyed connection object.
8f584bc 
there is a case that accessing a connection object which was destroyed by
ConnectionManager and TransportSubscriberLink/TransportPublicationLink
while getting a close event in pollset to call Connection::drop.

Signed-off-by: Barry Xu <Barry.Xu@sony.com>
@Barry-Xu-2018 Barry-Xu-2018 force-pushed the topic-fix-using-destroyed-connection branch from 28dfb94 to 8f584bc Compare May 18, 2020 02:35
@Barry-Xu-2018
Copy link
Contributor Author

@dirk-thomas

With #1940 merged this needs to be rebased. Please still clarify why this is (still) necessary.

Rebase action is done.

#1940 is to fix that calling drop_signal_() might access a TransportSubscriberLink/TransportPublicationLink which is already destroyed.
#1950 is to fix that calling Connection::drop() might access a Connection which is already destroyed

@dirk-thomas
Copy link
Member

Thanks for the patch as well as iterating on it.

@dirk-thomas dirk-thomas merged commit 03a7438 into ros:noetic-devel May 21, 2020
dirk-thomas pushed a commit that referenced this pull request May 21, 2020
@Barry-Xu-2018 @dirk-thomas
Fix a bug that using a destroyed connection object. (#1950)
e4b5b8e 
there is a case that accessing a connection object which was destroyed by
ConnectionManager and TransportSubscriberLink/TransportPublicationLink
while getting a close event in pollset to call Connection::drop.

Signed-off-by: Barry Xu <Barry.Xu@sony.com>
@dirk-thomas dirk-thomas mentioned this pull request May 21, 2020
Merged
@venkisagunner93
Copy link

venkisagunner93 commented Jun 12, 2020 •
edited
Loading

Just dropping by. I saw that the backporting is complete. If possible can someone tell me when the package will be available as a debian? I searched http://packages.ros.org/ros/ubuntu/pool/main/r/ros-melodic-ros-comm/ and I still see 1.14.5 not 1.14.6. Just wondering about the process and timeline as to when a pull request is made, how long until the package gets debianized? Trying to be understand the release cycles. @dirk-thomas

@dirk-thomas
Copy link
Member

Once a new version is tagged in the upstream repo it is released using bloom. The buildfarm will create new Debian packages shortly after that. Once all Debian packages for a ROS distro have been rebuild they are available in the testing repo (which you can use if you want to get new versions earlier). And every few weeks a sync is being made to the main repo most users use. That sync is announced on Discourse, e.g. https://discourse.ros.org/t/new-packages-for-melodic-2020-06-12/14640, which is also the annoucement mentioning the new version you are looking for.

@venkisagunner93
Copy link

@dirk-thomas Thank you. Appreciate it !

@chris20623 chris20623 mentioned this pull request Jun 23, 2020
Open
@dirk-thomas dirk-thomas mentioned this pull request Aug 4, 2020
Merged
dirk-thomas pushed a commit that referenced this pull request Aug 4, 2020
@Barry-Xu-2018 @dirk-thomas
Fix a bug that using a destroyed connection object. (#1950)
c4c303f 
there is a case that accessing a connection object which was destroyed by
ConnectionManager and TransportSubscriberLink/TransportPublicationLink
while getting a close event in pollset to call Connection::drop.

Signed-off-by: Barry Xu <Barry.Xu@sony.com>
nim65s added a commit to nim65s/robotpkg that referenced this pull request Mar 12, 2021
@nim65s
[middleware/ros-comm] Update to v1.15.9
96b5ec0 
Because DEPEND_ABI.ros-comm.noetic?= ros-comm>=1.15

1.15.9 (2020-10-16)
-------------------
* Fix deadlock when service connection is dropped (ros/ros_comm#2074)
* Update maintainers (ros/ros_comm#2075)
* Fix case where accessing cached parameters shuts down another node (ros/ros_comm#2068)
* Fix spelling (ros/ros_comm#2066)
* Fix Lost Wake Bug in ROSOutAppender (ros/ros_comm#2033)
* Fix compatibility issue with boost 1.73 and above (ros/ros_comm#2023)
* Gracefully stop recording upon SIGTERM and SIGINT (ros/ros_comm#2038)
* Use heapq.merge instead of custom merge sort code (ros/ros_comm#2017)
* Fix handling of single quotes in command arguments on Windows (ros/ros_comm#2051)
* Clearer error message (ros/ros_comm#2035)
* Ignore underscores when parsing literal numeric values for Python 3 compatibility (ros/ros_comm#2022)
* Clear cached URI for a node that has gone offline (ros/ros_comm#2010)
* Add skip_cache parameter to rosnode_ping() (ros/ros_comm#2009)
* Install advertisetest (ros/ros_comm#2046)
* Use range instead of xrange for Python 3 compatibility (ros/ros_comm#2013)
* Fix to address CVE-2020-16124 (ros/ros_comm#2065)
* Fix XmlRpcValue::_doubleFormat being unused (ros/ros_comm#2003)

1.15.8 (2020-07-23)
-------------------
* change is_async_connected to use epoll when available (ros/ros_comm#1983)
* allow mixing latched and unlatched publishers (ros/ros_comm#1991)
* remove not existing NodeProxy from rospy __all_\_ (ros/ros_comm#2007)
* fix typo in topics.py (ros/ros_comm#1977)
* fix bad relative import (still Python 2 style) (ros/ros_comm#1973)
* improve shutdown message with duplicate node name (ros/ros_comm#1992)
* remove dependency on rostopic from rostest package (ros/ros_comm#2002)
* fix missing reload() function in Python 3 (ros/ros_comm#1968)
* add latch param to throttle (ros/ros_comm#1944)
* add const versions of XmlRpcValue converting operators (ros/ros_comm#1978)

1.15.7 (2020-05-28)
-------------------
* fix Windows build break (ros/ros_comm#1961)
* fix NameError in launch error handling (ros/ros_comm#1965)

1.15.6 (2020-05-21)
-------------------
* fix a bug that using a destroyed connection object (ros/ros_comm#1950)

1.15.5 (2020-05-15)
-------------------
* check if async socket connect is success or failure before TransportTCP::read() and TransportTCP::write() (ros/ros_comm#1954)
* fix bug that connection drop signal related funtion throw a bad_weak exception (ros/ros_comm#1940)
* multiple latched publishers per process on the same topic (ros/ros_comm#1544)
* fix negative numbers in ros statistics (ros/ros_comm#1531)
* remove extra \n in ROS_DEBUG (ros/ros_comm#1925)
* add option to repeat latched messages at the start of bag splits (ros/ros_comm#1850)
* fix bag migration failures caused by typo in connection_header assignment (ros/ros_comm#1952)
* fix brief description comments after members (ros/ros_comm#1920)
* add --sigint-timeout and --sigterm-timeout parameters (ros/ros_comm#1937)
* roslaunch-check: search dir recursively (ros/ros_comm#1914)
* sort printed nodes by namespace alphabetically (ros/ros_comm#1934)
* remove pycrypto import (not used) (ros/ros_comm#1922)
* avoid infinite recursion in rosrun tab completion when rosbash is not installed (ros/ros_comm#1948)
* fix bare pointer in topic_tools::ShapeShifter (ros/ros_comm#1722)
* clear message queue on simtime jumping back (ros/ros_comm#1518)
* use undefined dynamic_lookup on macOS (ros/ros_comm#1923)
* check if enough FDs are free, instead counting the total free FDs (ros/ros_comm#1929)

1.15.4 (2020-03-19)
-------------------
* restrict boost dependencies to components used (ros/ros_comm#1871)
* add exception for ConnectionAbortedError (ros/ros_comm#1908)
* fix mac trying to use epoll instead of kqueue (ros/ros_comm#1907)
* fix AttributeError: __exit__ (ros/ros_comm#1915, regression from 1.14.4)

1.15.3 (2020-02-28)
-------------------
* remove Boost version check since Noetic only targets platforms with 1.67+ (ros/ros_comm#1903)

1.15.2 (2020-02-25)
-------------------
* export missing Boost dependency (ros/ros_comm#1898)
* add timestamp formatting for rosconsole (ros/ros_comm#1892)

1.15.1 (2020-02-24)
-------------------
* fix missing boost dependencies (ros/ros_comm#1895)
* use setuptools instead of distutils (ros/ros_comm#1870)
* increase time limit of advertisetest/publishtest.test to reduce flakyness (ros/ros_comm#1897)

1.15.0 (2020-02-21)
-------------------
* fix dictionary changed size during iteration (ros/ros_comm#1894)
* update test to pass with old and new yaml (ros/ros_comm#1893)

Packaging changes:
- removed patch-an, as there are no more boost version checks
- updated patch-ao
josephduchesne added a commit to josephduchesne/ros_comm that referenced this pull request Jan 24, 2024
@josephduchesne
backported ros#1950
4236c93
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjcarroll mjcarroll mjcarroll approved these changes

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees
No one assigned
Labels
bug consider-backport
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Barry-Xu-2018 @fujitatomoya @dirk-thomas @venkisagunner93 @mjcarroll