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

Keep track of fallthrough value to avoid false decrement #56

Closed
wants to merge 1 commit into from

Conversation

raJeev-M
Copy link

@raJeev-M raJeev-M commented May 28, 2019

Cause: #52

@ghost
Copy link

ghost commented May 28, 2019

loop->data.sweep_timer = us_create_timer(loop, 1, 0);

loop->data.wakeup_async = us_internal_create_async(loop, 1, 0);

Fallthrough polls are used pretty much only for internal timers and internal "asyncs" when creating a loop. It makes sense to only keep track/support fallthrough polls that are asyncs or timers, since those are rarely constructed.

@raJeev-M
Copy link
Author

@alexhultman, Thank you for the response. I've come across those two polls before making this change, but for some weird reason I thought like-- what if there might be more fallthrough polls added in the future? (looks like we won't be).

Off the top of my head, one solution which I am currently thinking that requires no change in the interface as well as any Type is: directly calling free() in both timer/async_close function, rather than doing it via us_poll_free.

@ghost
Copy link

ghost commented May 29, 2019

You can probably just make all polls increment the poll counter but if the poll is a fallthrough then also increment the threshold at where the loop falls through?

@ghost
Copy link

ghost commented May 29, 2019

So if you have 5 polls where 2 are fallthrough then loop has counters 5 and 2 and every free decrements the total but not the threshold

@raJeev-M
Copy link
Author

@alexhultman, I am thinking like, do we really need any change (adding threshold/ if..else) for the 2 fallthrough polls we are currently having? how about the below approach?

In us_create_poll() we do have an if statement for incrementing the 'num_polls', so, if we have two fallthrough polls, but zero non-fallthrough polls then the num_polls going to be zero, and with this being the case

we can add an one-line change under us_poll_free() like loop->num_polls? loop->num_polls-- : 0

would that be an acceptable change?

@raJeev-M raJeev-M closed this Jun 10, 2019
@raJeev-M raJeev-M deleted the bug#52 branch June 10, 2019 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant