p36850791
/
kinetic-devel
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix deadlock with self-subscriptions: remove blocking negotiateConnection(), which was a workaround for a previously-fixed bug.

This commit is contained in:
Josh Faust 2009-10-01 20:39:44 +00:00
parent 01bfa43bb8
commit c8a383408e
3 changed files with 9 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
core/roscpp/include/ros/subscription.h
View File

@ -79,9 +79,8 @@ public:
/** /**
* \brief Negotiates a connection with a publisher * \brief Negotiates a connection with a publisher
* \param xmlrpc_uri The XMLRPC URI to connect to to negotiate the connection * \param xmlrpc_uri The XMLRPC URI to connect to to negotiate the connection
* \param block If true, complete the connection negotiation before returning.
*/ */
bool negotiateConnection(const std::string& xmlrpc_uri, bool block); bool negotiateConnection(const std::string& xmlrpc_uri);
/** /**
* \brief Returns whether this Subscription has been dropped or not * \brief Returns whether this Subscription has been dropped or not
*/ */

34
core/roscpp/src/libros/subscription.cpp
View File

@ -242,7 +242,7 @@ bool Subscription::pubUpdate(const V_string& new_pubs)
// this function should never negotiate a self-subscription // this function should never negotiate a self-subscription
if (XMLRPCManager::instance()->getServerURI() != *i) if (XMLRPCManager::instance()->getServerURI() != *i)
{ {
retval &= negotiateConnection(*i, false); retval &= negotiateConnection(*i);
} }
} }
@ -257,8 +257,7 @@ bool Subscription::pubUpdate(const V_string& new_pubs)
return retval; return retval;
} }
bool Subscription::negotiateConnection(const std::string& xmlrpc_uri, bool Subscription::negotiateConnection(const std::string& xmlrpc_uri)
bool block)
{ {
XmlRpcValue tcpros_array, protos_array, params; XmlRpcValue tcpros_array, protos_array, params;
XmlRpcValue udpros_array; XmlRpcValue udpros_array;
@ -336,32 +335,11 @@ bool Subscription::negotiateConnection(const std::string& xmlrpc_uri,
// destruction. // destruction.
PendingConnectionPtr conn(new PendingConnection(c, udp_transport, shared_from_this(), xmlrpc_uri)); PendingConnectionPtr conn(new PendingConnection(c, udp_transport, shared_from_this(), xmlrpc_uri));
// Are we supposed to complete this connection in-place? (used for XMLRPCManager::instance()->addASyncConnection(conn);
// self-subscriptions) // Put this connection on the list that we'll look at later.
if(block)
{ {
ROS_DEBUG_NAMED("superdebug", "Making blocking connection to %s", boost::mutex::scoped_lock pending_connections_lock(pending_connections_mutex_);
xmlrpc_uri.c_str()); pending_connections_.insert(conn);
ROS_DEBUG_NAMED("superdebug", "Adding connection to http://%s:%d to server %p 's watch list",
c->getHost().c_str(),
c->getPort(),
&(c->_disp));
c->_disp.addSource(c, XmlRpc::XmlRpcDispatch::WritableEvent | XmlRpc::XmlRpcDispatch::Exception);
while(!conn->check())
{
ROS_DEBUG_NAMED("superdebug", "Waiting to complete connection to %s",
xmlrpc_uri.c_str());
c->_disp.work(0.01);
}
}
else
{
XMLRPCManager::instance()->addASyncConnection(conn);
// Put this connection on the list that we'll look at later.
{
boost::mutex::scoped_lock pending_connections_lock(pending_connections_mutex_);
pending_connections_.insert(conn);
}
} }
return true; return true;

4
core/roscpp/src/libros/topic_manager.cpp
View File

@ -364,7 +364,7 @@ bool TopicManager::advertise(const AdvertiseOptions& ops, const SubscriberCallba
if(found) if(found)
{ {
sub->negotiateConnection(xmlrpc_manager_->getServerURI(), true); sub->negotiateConnection(xmlrpc_manager_->getServerURI());
} }
XmlRpcValue args, result, payload; XmlRpcValue args, result, payload;
@ -471,7 +471,7 @@ bool TopicManager::registerSubscriber(const SubscriptionPtr& s, const string &da
s->pubUpdate(pub_uris); s->pubUpdate(pub_uris);
if (self_subscribed) if (self_subscribed)
{ {
s->negotiateConnection(xmlrpc_manager_->getServerURI(), true); s->negotiateConnection(xmlrpc_manager_->getServerURI());
} }
return true; return true;