Segfault after periodic activity deletion

[pdima]

I have a task with periodic activity created in updateHook():

bool Task::configureHook()
{
    setActivity(new RTT::Activity(ORO_SCHED_RT, m_cfg.priority, m_cfg.period));
    return true;
}

task.start();
....
task.stop(); task.configure(); task.start()

It always segfaults, looks like the activity loop continues to use the old activity data, deleted when the new Activity created. I tested with the current toolchain 2.9, but can reproduce the it with older versions as well.

Valgrind output:

==27410== Thread 7:
==27410== Invalid read of size 8
==27410==    at 0x6266C1C: RTT::os::MutexLock::MutexLock(RTT::os::MutexInterface&) (MutexLock.hpp:63)
==27410==    by 0x795865B: RTT::Activity::loop() (Activity.cpp:211)
==27410==    by 0x79681B0: RTT::os::thread_function(void*) (Thread.cpp:100)
==27410==    by 0x796CA0F: RTT::os::rtos_posix_thread_wrapper(void*) (fosi_internal.cpp:99)
==27410==    by 0x96F4181: start_thread (pthread_create.c:312)
==27410==    by 0xA22447C: clone (clone.S:111)
==27410==  Address 0xe625180 is 256 bytes inside a block of size 368 free'd
==27410==    at 0x4C2C2BC: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27410==    by 0x7958267: RTT::Activity::~Activity() (Activity.cpp:110)
==27410==    by 0x79051DD: void boost::checked_delete<RTT::base::ActivityInterface>(RTT::base::ActivityInterface*) (checked_delete.hpp:34)
==27410==    by 0x7918547: boost::detail::sp_counted_impl_p<RTT::base::ActivityInterface>::dispose() (sp_counted_impl.hpp:78)
==27410==    by 0x5FA7301: boost::detail::sp_counted_base::release() (sp_counted_base_gcc_x86.hpp:146)
==27410==    by 0x5FA73C4: boost::detail::shared_count::~shared_count() (shared_count.hpp:371)
==27410==    by 0x78FD6E9: boost::shared_ptr<RTT::base::ActivityInterface>::~shared_ptr() (shared_ptr.hpp:328)
==27410==    by 0x78FEB91: boost::shared_ptr<RTT::base::ActivityInterface>::reset() (shared_ptr.hpp:619)
==27410==    by 0x78FCA56: RTT::TaskContext::setActivity(RTT::base::ActivityInterface*) (TaskContext.cpp:350)
==27410==    by 0x17F24A70: Task::configureHook()

[meyerj]

I cannot reproduce this issue with 2.9 and the gnulinux target.

With https://github.com/orocos-toolchain/rtt/commit/58cb4bb733fc14de0ca520620cfcb689e2155797 it should not be possible to destroy an Activity instance before the underlying thread is terminated. Before 2.9 the thread function was implemented in the RTT::os::Thread class and it was only the ~Thread destructor that joined after the Activity subclass has already been destroyed.

I see that there might be issues if setActivity() would be called from the thread to be destroyed itself. Not sure if this is properly caught. But that does not seem to be the case in your example. Which thread executes the stop, configure, start operations? The TaskBrowser? Or a script running in the scripting service of exactly the component in question?

If there is no need to recreate the activity because only the periodicity changed, prefer setPeriod() instead, which works even while the activity is running.

[pdima]

I found task.stop(); task.configure(); task.start(); sequence is called in client thread as OwnThread operation. It works fine when ClientThread used.

Is it acceptable to change activity during configureHook? I tried to apply priority and scheduler from properties/configuration not available at construction time.

[meyerj]

Is it acceptable to change activity during configureHook? I tried to apply priority and scheduler from properties/configuration not available at construction time.

In general it is okay to set the component's activity in the configureHook(), but only if it is called from an external thread, not from the thread running the component itself (like in an OwnThread operation). Usually this external thread is the main thread and configure() is called during deployment.

You can modify all relevant properties of an existing activity in configureHook(), while the thread is running without having to destroy the activity:

RTT::TaskContext *tc = this;
tc->setPeriod(new_period);
RTT::os::ThreadInterface *thread = tc->getActivity()->thread();
if (thread) {
    thread->setScheduler(sched_type);
    thread->setPriority(priority);
}

Note that not every activity has its own thread and a thread might be reused by multiple activities for certain activity types. At the moment getActivity()->thread() always returns the main thread in case there is no thread associated to the activity, but I am going to change that for 2.9. For the standard Activity there are no issues and the code snippet above should just do what you would expect.