Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed blocking calls to ExecutionEngine::process() #3

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions rtt/Activity.cpp
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ namespace RTT
Activity::~Activity() Activity::~Activity()
{ {
stop(); stop();

// We need to join the activity's thread before destruction as the thread function might still
// access member variables. Activity::stop() does not guarantuee to stop the underlying thread.
terminate();
} }


os::ThreadInterface* Activity::thread() { os::ThreadInterface* Activity::thread() {
Expand Down Expand Up @@ -142,7 +146,10 @@ namespace RTT
if ( ! Thread::isActive() ) if ( ! Thread::isActive() )
return false; return false;
//a trigger is always allowed when active //a trigger is always allowed when active
msg_cond.broadcast(); {
os::MutexLock lock(msg_lock);
msg_cond.broadcast();
}
Thread::start(); Thread::start();
return true; return true;
} }
Expand All @@ -158,7 +165,6 @@ namespace RTT
return false; return false;
} }
mtimeout = true; mtimeout = true;
msg_cond.broadcast();
Thread::start(); Thread::start();
return true; return true;
} }
Expand Down
24 changes: 14 additions & 10 deletions rtt/ExecutionEngine.cpp
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ namespace RTT
assert(foo); assert(foo);
if ( foo->execute() == false ){ if ( foo->execute() == false ){
foo->unloaded(); foo->unloaded();
os::MutexLock lock(msg_lock);
msg_cond.broadcast(); // required for waitForFunctions() (3rd party thread) msg_cond.broadcast(); // required for waitForFunctions() (3rd party thread)
} else { } else {
f_queue->enqueue( foo ); f_queue->enqueue( foo );
Expand Down Expand Up @@ -211,16 +212,16 @@ namespace RTT
assert( com ); assert( com );
com->executeAndDispose(); com->executeAndDispose();
} }
// there's no need to hold the lock during
// emptying the queue. But we must hold the
// lock once between excuteAndDispose and the
// broadcast to avoid the race condition in
// waitForMessages().
// This allows us to recurse into processMessages.
MutexLock locker( msg_lock );
} }
if ( com )
msg_cond.broadcast(); // required for waitForMessages() (3rd party thread) // there's no need to hold the lock during
// emptying the queue. But we must hold the
// lock once between excuteAndDispose and the
// broadcast to avoid the race condition in
// waitForMessages().
// This allows us to recurse into processMessages.
MutexLock locker( msg_lock );
msg_cond.broadcast(); // required for waitForMessages() (3rd party thread)
} }


bool ExecutionEngine::process( DisposableInterface* c ) bool ExecutionEngine::process( DisposableInterface* c )
Expand All @@ -237,7 +238,10 @@ namespace RTT


bool result = mqueue->enqueue( c ); bool result = mqueue->enqueue( c );
this->getActivity()->trigger(); this->getActivity()->trigger();
msg_cond.broadcast(); // required for waitAndProcessMessages() (EE thread) {
os::MutexLock lock(msg_lock);
msg_cond.broadcast(); // required for waitAndProcessMessages() (EE thread)
}
return result; return result;
} }
return false; return false;
Expand Down
1 change: 1 addition & 0 deletions rtt/os/Thread.cpp
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ namespace RTT {
rtos_sem_signal(&sem); rtos_sem_signal(&sem);


rtos_task_delete(&rtos_task); // this must join the thread. rtos_task_delete(&rtos_task); // this must join the thread.
active = false;
} }


const char* Thread::getName() const const char* Thread::getName() const
Expand Down