Replace select(2) with poll(2) #1991
Merged
Conversation
rsmarples
force-pushed the
rsmarples:poll
branch
2 times, most recently
from
6749989
to
ede7e9d
|
Thanks for submitting this. I'm still looking at the changes. Though, would you be able to run it through valgrind, so far I'm getting several Invalid reads on my end, wondering if you are seeing the same on your side? Thank you! |
|
I'll find some time for this. |
|
I'll take a look at it as well and see what I find. Thanks again! |
|
PR #1993 is required as the first stepping stone to get address sanitisation to work via the clang compiler. |
poll(2) is more modern, more efficient and a lot faster than select(2). The trade-off is that we need to manage the array of struct pollfd we send it a bit more intelligently than just manipulating tables before each call to select(2). This commit adds functions to dynamically resize the allocation needed when new fd's need to be monitored or old ones discarded and then sprinkles them where needed.
|
Should be fixed now. |
|
Merged. Thanks again for fixing this and submitting this PR. I added a fix (98d6947) for a leak on the last fdstate. Let me know if it makes sense adding that free call at point. |
So that means that an fd being monitored wasn't closed. Working out which fd isn't being closed and ensuring it is would be the cleaner approach if you feel that's important. If you ever have plans to open anothe websocket afterwards, you might want to set vars to NULL after freeing them, especially static ones such as fdstate. |
|
I see what you mean, yeah it's definitely not a biggie as it appears it was only upon stopping the server, though, mostly so it doesn't raise any flags on my tests :) I just added a minor commit to NULL in case we ever need to restart it. |
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.
poll(2) is more modern, more efficient and a lot faster than
select(2). The trade-off is that we need to manage the
array of struct pollfd we send it a bit more intelligently
than just manipulating tables before each call to select(2).
This commit adds functions to dynamically resize the allocation
needed when new fd's need to be monitored or old ones discarded
and then sprinkles them where needed.