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

Bug: circular reference in callback_timer_* when start() is called inside callback for repeating timers #967

Open
ronaldd2 opened this issue Oct 7, 2024 · 5 comments
Assignees
Labels

Comments

@ronaldd2
Copy link

ronaldd2 commented Oct 7, 2024

The callback_timer_locked, callback_timer_atomic and callback_timer_interrupt is restarting the timer automatically after the callback is handled for repeating timers. If inside the callback the period is changed the timer is stopped, so normally you should call a start timer to start it again. When doing this from within in the callback, the timer is inserted twice in the list and a circular reference is created in the linked list.

The superseded callback_timer doesn't have this problem as there the timer is inserted before the callback is called. I see this behavior is changed 2 years ago in the other versions. Can you remember what the rationale for this change was?

Maybe changing the period inside the callback for a repeating timer is not intended, but this is not documented. Also it's not possible to stop a timer in the callback as it will be started automatically after the call back.

The easiest fix, I think, is to skip the automatic insert if the timer.is_active() is set, as normally it not possible to do an insert twice.
Moving the insert back is not a real option I think as this gives totally different behavior for existing code.

Thanks in advance

@KammutierSpule
Copy link

Hi @ronaldd2 would you like to have a look to see if this is related or duplicated of
#954

@ronaldd2
Copy link
Author

ronaldd2 commented Oct 7, 2024

There is some relation, the unit test can indeed trigger this exact flow as the unit test is starting it's own timer in the callback.

@KammutierSpule
Copy link

There is also this one:

#952

because "If inside the callback the period is changed the timer is" changed, and you are using a mutex protected timer, then you will get a deadlock.

@itavero
Copy link

itavero commented Nov 5, 2024

The superseded callback_timer doesn't have this problem as there the timer is inserted before the callback is called. I see this behavior is changed 2 years ago in the other versions. Can you remember what the rationale for this change was?

@jwellbelove Can you perhaps shed a light on this?

@jwellbelove
Copy link
Contributor

I'll take a look as soon as I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

4 participants