use 2x buffer when sysctl fails with ENOMEM#591
Merged
wfurt merged 2 commits intodotnet:masterfrom Dec 6, 2019
Merged
Conversation
lpereira
approved these changes
Dec 5, 2019
Member
|
3.1 servicing candidate maybe? |
Member
Author
|
yes, I was thinking about it as the failure mode is rather unpleasant and there is no workaround. |
stephentoub
reviewed
Dec 6, 2019
| } | ||
|
|
||
| byteCount = tmpEstimatedSize; | ||
| buffer = realloc(buffer, byteCount); |
Member
There was a problem hiding this comment.
Can you add a comment (similar to your PR description) as to why we're not just using byteCount as returned by sysctl?
stephentoub
approved these changes
Dec 6, 2019
Contributor
|
@wfurt it's unclear to me why I was CC'd, does this relate to some work I've done that I'm forgetting about? |
Member
Author
|
sorry fir the noise @ryanbrandenburg. I cc you because it is in same area as https://github.com/dotnet/corefx/issues/41740 you opened while back. |
|
Thanks for the quick fix. Is this in fix in the latest Master (5.0.x Runtime)? Can I download the installer (5.0.100-alpha1-015914) to test the fix now? Do you have an ETA for the 3.1 servicing pack? |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
https://github.com/dotnet/corefx/issues/42634 is caused by endless spin in System.Native when we fail to process routing table on first try. It was reported only on Catalina but that may be just timing difference.
It seems like existing code assumes that when sysctl() fails with ENOMEM, oldlenp would have new needed size. But that is not happening. According to 10.14 man page:
I verified that if sysctl fails with ENOMEM and provided buffer, oldp has still old value e.g. amount written. I also verified that if we do another sysctl() call with buffer set to NULL, it provides back updated estimate. That however seems expensive as we need context switch to kernel and kernel will need to process routing table to get new estimate. Instead I decided to use same strategy as we use to dump tcp socket list and simply increase buffer 2x. That may temporarily need more memory but should be much cheeper on CPU processing.
cc: @ryanbrandenburg
fixes https://github.com/dotnet/corefx/issues/42634