Finally fixing the SlaveActivity message processing #71
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…an ExecutionEngine Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
…nning in a SlaveActivity Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
…ProcessMessages() calls to master engine waitForMessagesInternal() is also used by waitForFunctions() and thus would wait for the wrong condition variable. Forwarding waitForMessages() to the master should be sufficient for processing operations in slave engines. Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
…cutionEngine::process() The forwarding does not require that the slave engine has an activity. Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
smits
pushed a commit
that referenced
this pull request
Nov 13, 2014
Finally fixing the SlaveActivity message processing
|
Sorry forgot to reply, this fix is working fine for my testcase. |
Merged
meyerj
added a commit
that referenced
this pull request
May 8, 2019
The patch reverts most of the previous patches applied to fix the issue that `OwnThread` operations provided by components running in a SlaveActivity have only been executed when the slave was updated, but never asynchronously like for normal activities. This caused dead-locks in certain situations when calling operations of slaves that are never updated before the call returns. The history of the reverted patches started with #35, with follow-ups in https://github.com/orocos-toolchain/rtt/issue/59, #60 and #71. The `slave_test` added in one of the original PRs was not reverted and still succeeds with the new patch. The new solution to the original problem is much less intrusive: Instead of forwarding the operations calls itself and making the ExecutionEngine aware of the existence of masters and slaves, the SlaveActivity that gets triggered (e.g. as a consequence of an operation call) enqueues a message in the master's ExecutionEngine that processes the triggers in the slave's ExecutionEngine `work(Trigger)` callback, which then processes all the pending messages (and port callbacks) of the slave. This also works across multiple levels of master/slave relations. The difference to the previous patch is the pending messages are still enqueued in the engine of the actual runner and explicit calls to `Slave.update()` from the master thread will also process the pending operations of the slave - but not of the master. Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
meyerj
added a commit
that referenced
this pull request
May 8, 2019
The patch reverts most of the previous patches applied to fix the issue that `OwnThread` operations provided by components running in a SlaveActivity have only been executed when the slave was updated, but never asynchronously like for normal activities. This caused dead-locks in certain situations when calling operations of slaves that are never updated before the call returns. The history of the reverted patches started with #35, with follow-ups in https://github.com/orocos-toolchain/rtt/issue/59, #60 and #71. The `slave_test` added in one of the original PRs was not reverted and still succeeds with the new patch. The new solution to the original problem is much less intrusive: Instead of forwarding the operations calls itself and making the ExecutionEngine aware of the existence of masters and slaves, the SlaveActivity that gets triggered (e.g. as a consequence of an operation call) enqueues a message in the master's ExecutionEngine that processes the triggers in the slave's ExecutionEngine `work(Trigger)` callback, which then processes all the pending messages (and port callbacks) of the slave. This also works across multiple levels of master/slave relations. The difference to the previous patch is the pending messages are still enqueued in the engine of the actual runner and explicit calls to `Slave.update()` from the master thread will also process the pending operations of the slave - but not of the master. Signed-off-by: Johannes Meyer <johannes@intermodalics.eu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #35, @goldhoorn reported an issue with components that run in a SlaveActivity and proposed a patch to solve it. Calls to OwnThread operations of such a component have blocked until the next
execute()call, which probably never happened if the master was not running. #60 partially reverted this patch and provided a different solution because the original patch broke use cases of SlaveActivities that do not have a master (reported in #59). Unfortunately, #60 was ineffective and only restored the original state.This pull request hopefully fixes this issue following the comments on commit 2a012d6 and adds a new unit test to test operation calls to slave components with and without callbacks.