FileDescriptorActivity locking and robustness #309
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
* do not signal the command pipe if the trigger or break flag was already set * make sure that the IOReady step cannot be executed if the command pipe was the only active file descriptor * fixed return value of hasTimeout(): the function should return true if the current execution cycle was triggered due to a select timeout Signed-off-by: Johannes Meyer <[email protected]>
…to a trigger() call Signed-off-by: Johannes Meyer <[email protected]>
m_running was reset to false after the first call to step() and before a call to work(...). Signed-off-by: Johannes Meyer <[email protected]>
…t locks in trigger() call Signed-off-by: Johannes Meyer <[email protected]>
…ndFlags() Signed-off-by: Johannes Meyer <[email protected]>
Signed-off-by: Johannes Meyer <[email protected]>
…tect the set of watched file descriptors Signed-off-by: Johannes Meyer <[email protected]>
…rrent removal of watched fd and write Signed-off-by: Johannes Meyer <[email protected]>
meyerj
force-pushed
the
master-file-descriptor-activity-locking-and-robustness
branch
from
96eced7
to
cecf831
Compare
doudou
reviewed
May 7, 2020
Comment on lines
+329
to
+334
| if (m_has_timeout || m_has_ioready || m_has_error || | ||
| (timeout.tv_sec == 0 && timeout.tv_usec == 0)) { | ||
| static const int USECS_PER_SEC = 1000000; | ||
| timeout.tv_sec = m_timeout_us / USECS_PER_SEC; | ||
| timeout.tv_usec = m_timeout_us % USECS_PER_SEC; | ||
| } |
There was a problem hiding this comment.
Would be good to add a unit test for this one.
17 tasks
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.
This PR collects some patches to improve robustness of the
FileDescriptorActivityand to avoid locks intrigger()andbreakLoop()calls.FileDescriptorActivity::trigger(),FileDescriptorActivity::breakLoop()andFileDescriptorActivity::triggerUpdateSets()(contributed by @MagnaboscoL)trigger()call. The previous implementation was waiting for the set timeout period after every interruption, which in theory could lead to the situation that the timeout cycle is never triggered if there is no activity on the file descriptor but the activity is constantly triggered due to operation calls or port callbacks.m_runningflag was buggy andisRunning()might have returned false even though the activity was still processing thestep()function.