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

process: implement NumFDs for Windows #1454

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

braydonk
Copy link

This PR implements the NumFDs method for Windows by borrowing the system info query from OpenFiles and using it to count any Handles belonging to the process.

This PR implements the `NumFDs` method for Windows by borrowing the system info
query from `OpenFiles` and using it to count any Handles belonging to
the process.

Signed-off-by: braydonk <braydonk@google.com>
@braydonk braydonk marked this pull request as draft April 27, 2023 15:59
@shirou
Copy link
Owner

shirou commented May 28, 2023

Thank you for your PR. It works on my Windows 10, at least return some values. However, after some searching, I found GetProcessHandleCount. With my knowledge I do not know which is better than your PR. Could you please provide us with some URLs or other references about this PR? Thank you so much!

@braydonk
Copy link
Author

Hi @shirou thanks for taking a look!

I think you're right, since this PR I've figured out a lot better ways to get this information. In this library in particular, I believe GetProcessHandleCount would indeed be the better choice given that it is designed to operate one process at a time.

I can make that change. In the accompanying issue #1455 I asked a question about the naming for this API; the name NumFDs actually wouldn't be very descriptive for this particular use case, because on Windows, handles held by a process are a lot more than just file descriptors. Is there a way that we could name this API more clearly? It looks like the current design is to have the same interface on all platforms, or is there precedent for breaking that somewhere I'm not seeing?

@shirou
Copy link
Owner

shirou commented May 29, 2023

NumFDs is comes from psutil. psutil has a num_handles for Windows. However, in go, we should use same name for all os, so I choose NumFD.

And we can not change the name because of compatibility.

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

Successfully merging this pull request may close these issues.

2 participants