ros / ros_comm

ROS communications-related packages, including core client libraries (roscpp, rospy, roslisp) and graph introspection tools (rostopic, rosnode, rosservice, rosparam).
http://wiki.ros.org/ros_comm
747 stars 914 forks source link

Segmentation fault when calling subscriber shutdown #2319

Closed lprobsth closed 1 year ago

lprobsth commented 1 year ago

I'm using ROS noetic and building a universal serialization program for bridging our custom message transport system with ROS. For converting ROS messages to our transport system, I'm using a universal subscriber with the help of RosMsgParser. Nodes can add or remove bridges for certain topics during runtime. The creation of new subcribers during runtime works fine and the bridge also works as expected.

When I try to delete Subscribers during removal of a bridge connection, I get a segmentation fault.

0x00007ffff7ec5664 in ros::Subscriber::shutdown() () from /opt/ros/noetic/lib/libroscpp.so

The following code shows a minimal example.

ros::Subscriber * g_subscr_ptr;

MyClass::MyClass(ros::NodeHandle *p_nodeHandle, [... other parameters including msg definition])
{
            RosMsgParser::ShapeShifter * l_shapeShifterPtr = new RosMsgParser::ShapeShifter();

            l_shapeShifterPtr->morph(l_msgDef->md5,
                                                            l_connectionParam->msgName,
                                                            l_msgDef->msgDef);

            std::string l_topicName = l_connectionParam->topicName;

            boost::function<void(const RosMsgParser::ShapeShifter::ConstPtr&) > callback;
            callback = [this,l_topicName](const RosMsgParser::ShapeShifter::ConstPtr& msg)
            {
                universalSubscriberCallback(msg, l_topicName, this) ;
            };

            ros::SubscribeOptions options;

            // populate the options object with basic subscriber info
            // this is extented in the next operation with the specified datatype and md5 hash
            options.init(l_topicName,
                15,
                                callback);

            options.md5sum = l_shapeShifterPtr->getMD5Sum();       // this is needed so rostopic info shows correct info without another publisher
            options.datatype = l_shapeShifterPtr->getDataType();   // ^^

            // subscribe with this data
            g_subscr_ptr = new ros::Subscriber(g_nodeHandle->subscribe(options));
}

void MyClass::universalSubscriberCallback( const RosMsgParser::ShapeShifter::ConstPtr& msg,const std::string &topic_name,MyClass * p_parentClass) {
            // process message
}

MyClass::removeConnection() {
    g_subscr_ptr->shutdown();
}

Problems I have in mind right now:

Did anybody use the shutdown function of Subscriber or run into a similar problem? Is there anything more I could provide (e.g. building ros_comm with debug symbols)?

rgmann commented 1 year ago

In the constructor, it looks like you are allocating a new Subscriber and assigning it to a local Subscriber variable called g_subscr_ptr, rather than to the global Subscriber at the top of the snippet. Is that just a copy/paste error in your example? Calling shutdown on an invalid pointer could certainly cause a segfault.

lprobsth commented 1 year ago

I'm really sorry - the shutdown function of the Subscriber works as expected.

My code example was an extract from a much bigger program - the pointer where I stored the Subscriber was sometimes not initialized and then pointed into random locations. I did check if the pointer is Null - but this check was not sufficient in that case. So my program tried to unsubscribe with a non existent Subscriber which led to the segfault.