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

[rospy] made get_published_topics threadsafe#958

Merged
dirk-thomas merged 2 commits intoros:kinetic-develfrom
jabbenseth:kinetic-devel
Jan 10, 2017
Merged

[rospy] made get_published_topics threadsafe#958
dirk-thomas merged 2 commits intoros:kinetic-develfrom
jabbenseth:kinetic-devel

Conversation

@jabbenseth
Copy link
Copy Markdown
Contributor

@jabbenseth jabbenseth commented Jan 10, 2017

With multiple threads trying to get the currently published topics at least one of them crashes with

Exception in thread Thread-4:
Traceback (most recent call last):
    topics = rospy.get_published_topics()
  File "/ros_comm/clients/rospy/src/rospy/client.py", line 376, in get_published_topics
    code, msg, val = get_master().getPublishedTopics(namespace)
  File "/ros_comm/clients/rospy/src/rospy/msproxy.py", line 106, in wrappedF
    return f(*args, **kwds)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1233, in __call__
    return self.__send(self.__name, args)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1587, in __request
    verbose=self.__verbose
  File "/usr/lib/python2.7/xmlrpclib.py", line 1273, in request
    return self.single_request(host, handler, request_body, verbose)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1303, in single_request
    response = h.getresponse(buffering=True)
  File "/usr/lib/python2.7/httplib.py", line 1077, in getresponse
    raise ResponseNotReady()
ResponseNotReady

Exception in thread Thread-5:
Traceback (most recent call last):

    topics = rospy.get_published_topics()
  File "/ros_comm/clients/rospy/src/rospy/client.py", line 376, in get_published_topics
    code, msg, val = get_master().getPublishedTopics(namespace)
  File "/ros_comm/clients/rospy/src/rospy/msproxy.py", line 106, in wrappedF
    return f(*args, **kwds)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1233, in __call__
    return self.__send(self.__name, args)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1587, in __request
    verbose=self.__verbose
  File "/usr/lib/python2.7/xmlrpclib.py", line 1273, in request
    return self.single_request(host, handler, request_body, verbose)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1298, in single_request
    self.send_request(h, handler, request_body)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1400, in send_request
    connection.putrequest("POST", handler, skip_accept_encoding=True)
  File "/usr/lib/python2.7/httplib.py", line 906, in putrequest
    raise CannotSendRequest()
CannotSendRequest

the acquisition of f is thread safe, but the final call to f seems not.

@jspricke
Copy link
Copy Markdown
Member

jspricke commented Jan 10, 2017

HI @ipa-jba thanks for the pull request, do you have a simple example to play around?
Looking good at first sight, @dirk-thomas any comments, as you did the implementation?

@jabbenseth
Copy link
Copy Markdown
Contributor Author

jabbenseth commented Jan 10, 2017 •
edited
Loading

on my machine this script crashes (without modifications) ten out of ten times:

#!/usr/bin/env python

import rospy

rospy.init_node("test_threadsafeness")

for i in range(2):
    rospy.Timer(rospy.Duration(1), rospy.get_published_topics)

rospy.spin()

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Jan 10, 2017

I can reproduce the problem as well as confirm that the patch makes this case pass. Since the proxy class claims that all methods are thread-safe (

ros_comm/clients/rospy/src/rospy/msproxy.py

Line 80 in 6a0672c

All methods are thread-safe.
) it makes sense to lock for the actual call.

To avoid the need for two separate locks the operation to get the function from target could be delayed until the actual call which then could be done together in a single lock:

with self._lock:
    f = getattr(self.target, key)
    return f(*args, **kwds)

@jabbenseth
Copy link
Copy Markdown
Contributor Author

jabbenseth commented Jan 10, 2017

I updated the code to implement your suggestions @dirk-thomas. One critical information for your review: I tested it on indigo and did an "untested" port to kinetic for this

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Jan 10, 2017

I tested it in Kinetic and CI also runs for Kinetic on this PR. I will wait with the merge until the new CI build passes. Thank you for the patch!

@dirk-thomas dirk-thomas merged commit 5c8997e into ros:kinetic-devel Jan 10, 2017
ggallagher01 pushed a commit to clearpathrobotics/ros_comm that referenced this pull request Jan 26, 2017
@ggallagher01
[rospy] made get_published_topics threadsafe (ros#958)
3596b17 
* [rospy] made get_published_topics threadsafe
@dirk-thomas dirk-thomas mentioned this pull request Mar 2, 2017
Merged
dirk-thomas pushed a commit that referenced this pull request Mar 2, 2017
@dirk-thomas
[rospy] made get_published_topics threadsafe (#958)
48ef2a1 
* [rospy] made get_published_topics threadsafe
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jabbenseth @jspricke @dirk-thomas