Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Refactor posix timer so that it uses only iceoryx_utils concurrent constructs #170

Closed
elfenpiff opened this issue Jul 3, 2020 · 2 comments
Labels
bug Something isn't working refactoring Refactor code without adding features test A module/integration/stress/etc test for a component

Comments

@elfenpiff
Copy link
Contributor

Required information

At the moment the timer uses multiple atomics, mutexes etc. to ensure that there is no race condition which would skip a callback or call it multiple times. The current implementation is too complex and we have no hard stress tests to make sure such race conditions do not exist anymore.

Hence, we have to refactor the timer so that it is based only on iceoryx_utils concurrent structures which are well tested. This would reduce the imminent risk of further race conditions or other concurrent misbehaviors massively.

@elfenpiff elfenpiff added bug Something isn't working refactoring Refactor code without adding features test A module/integration/stress/etc test for a component labels Jul 3, 2020
@elBoberido
Copy link
Member

An idea to simplify the timer would be to not spawn a new thread for each timer invocation, since this makes it really messy and introduces unnecessary complexity. Instead of SIGEV_THREAD for our sigevent we could probably use SIGEV_THREAD_ID on linux and SIGEV_SIGNAL_THREAD or SIGEV_PULSE on QNX. The callback would be executed after a blocking wait with sigwait or sigwaitinfo on a thread spawned by the user. I wouldn't recommend to let the timer spawn the thread, since this makes it less flexible.
This should massively reduce the complexity of the current implementation. I need to dig deeper into this to confirm if we could fully reimplement the timer with those sigevents.

@mossmaurice mossmaurice modified the milestone: v1.x Nov 4, 2020
@mossmaurice
Copy link
Contributor

Decision was to remove the posix::Timer completely and then start from scratch if there is the necessity for a timer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working refactoring Refactor code without adding features test A module/integration/stress/etc test for a component
Projects
None yet
Development

No branches or pull requests

3 participants