[SOLVED] Thread does not terminate

asked 2019-06-27 10:38:40 -0600

16 ●2 ●4 ●5

updated 2019-07-16 05:43:54 -0600

Hello, I've a problem with terminating a node and I hope someone can help. Consider some code like below:

unsigned char interestingNumber;
std::mutex mu;
std_msgs::UInt16 msgUInt;

// Function prototypes
void threadCallback(const ros::Publisher* const);
void subscriberCallback(const std_msgs::Float32MultiArray::ConstPtr&);

int main(int argc, char **argv)
{
    ros::init(argc, argv, "someNode");
    ros::NodeHandle n;
    ros::Publisher pub = n.advertise<std_msgs::UInt16>("topic", 1);
    ros::Subscriber sub = n.subscribe("interestingtopic", 10, subscriberCallback);
    msgUInt.data = 1;
    std::thread someThread(threadCallback, &pub);

    ros::spin();
    someThread.join();
    return 0;
}

void subscriberCallback(const std_msgs::Float32MultiArray::ConstPtr &msg)
{
    mu.lock();
    interestingNumber = msg->layout.dim[2].size;
    mu.unlock();
}

void threadCallback(const ros::Publisher* const p)
{
    ros::Rate loopRateInner(1000);
    while(ros::ok())
    {
        if(interestingNumber == 0)
            msgUint.data = 1;

        while(interestingNumber == 0)
        {
            if(msgUInt.data == 2000)
                msgUInt.data = 1;

            msgUInt.data++;

            p->publish(msgUInt);
            loopRateInner.sleep();
        }
    }
}

So what this basically does is, it subscribes to a topic and assigns a variable with a value of the messages on that subscribed topic. Asynchronously to that another thread has to publish to another topic depending on that received value. This also works fine. My problem is, that the thread someThread does not terminate as soon as I shut down all other nodes and the roscore. Also CTRL+C doesn't help. I always have to kill this node via the system manager. And I wonder why this happens, since the outer loop should stop executing as soon as ros::ok() returns false.

How can I fix this?

Thanks in advance for any help!

answered 2019-06-27 11:05:55 -0600

Zacryon

16 ●2 ●4 ●5

updated 2019-07-17 03:41:22 -0600

I solved the problem by myself when I checked again if I started the additional thread correctly at all. And it was a simple and stupid mistake.

Solution:

Just delete the line with someThread.join() and use someThread.detach() after intializing the thread and before calling ros::spin(). Detaching the additional thread ensures that the main thread and the additional one don't block each other.

So the main function looks like the following:

int main(int argc, char **argv)
{
    ros::init(argc, argv, "someNode");
    ros::NodeHandle n;
    ros::Publisher pub = n.advertise<std_msgs::UInt16>("topic", 1);
    ros::Subscriber sub = n.subscribe("interestingtopic", 10, subscriberCallback);
    msgUInt.data = 1;
    std::thread someThread(threadCallback, &pub);
    someThread.detach();

    ros::spin();
    return 0;
}

Now the node terminates as expected.

I'm still not completely sure why the problem occurs in the first place. Because as I understand it, ros:spin() will return to the calling method (here the main method) as soon as the node should shutdown. Why does the thread not 'join"?

EDIT: I forgot to update my solution. See comments below this answer for details. I also replaced the inner while loop with an if statement, so it looks something like this:

while(ros::ok())
{
    mu.lock()
    if(interestingNumber == 0)
    {
        if(msgUInt.data == 2000)
            msgUint.data = 1;

        msgUInt.data++;

        p->publish(msgUInt);
    }
    mu.unlock();
    loopRateInner.sleep();
}

edit flag offensive delete link

Comments

You solved it by not caring if the thread terminates correctly.

I'm guessing your thread gets stuck on the inner loop. interestingNumber is only updated by your callback, which isn't called after spin() returns. Adding a && ros::ok() to the while loop should make it quit.

Side note: Using the mutex only in the callback does not add any thread safety. You'll need to use it every time you access interestingNumber.

pcoenen ( 2019-07-16 08:10:34 -0600 )edit

Thank you for your comment! :) This is in some part correct. Though the detachement of the thread solved my original problem in a not so nice way to ensure that the thread can terminate, I did not bother if it terminates correctly. However, I got rid of the inner while loop a while ago and replaced it with an if statement. I forgot to update that in my answer. The structure looks now somewhat like this:

while(ros::ok())
{
    mu.lock()
    if(interestingNumber == 0)
    {
        if(msgUInt.data == 2000)
            msgUint.data = 1;

        msgUInt.data++;

        p->publish(msgUInt);
    }
    mu.unlock();
    loopRateInner.sleep();
}

That increased stability, since the whole loop body now only depends on ros::ok() and no longer on another condition, which may or may be not set by the callback function.

Thanks for your hint about the mutex, which I did not add to the read operation within the loop. I forgot about

Zacryon ( 2019-07-17 03:31:38 -0600 )edit

You don't need to lock for the whole if-term, if you create a local copy of your interestingNumber. Also have a look at scoped locks like lock_guard in C++11. Locking and unlocking manually can result in leaking mutexes due to early returns or exceptions.

Is your updated thread joinable?

pcoenen ( 2019-07-17 04:00:50 -0600 )edit

add a comment

[SOLVED] Thread does not terminate

1 Answer

Comments

Question Tools

Stats

Related questions

[SOLVED] Thread does not terminate edit

1 Answer

Comments

Question Tools

Stats

Related questions

[SOLVED] Thread does not terminate