Has some optimizations for closing all the files. If we can poke /proc/{pid}/fd, then we grab it, then store the max of the result +2 (because of those pesky exclusive ranges).
With the /proc/ stuff, it doesn't end up with the really big range, and my raspi blows through it in milliseconds.
Without the /proc/ optimization, my raspberry pi closes from [0->2] and then [5->17], and then again from [19->1048576]. The last range takes it about 3s to close, and calculation is negligibly slower since the last range doesn't require a bunch of iteration.
It could be argued the /proc/ stuff isn't as reliable or might be risky, but procfs is pretty reliable. If it can't be accessed for some reason, it'll be skipped and won't impact the current code.
Also, I fixed setup.py
I'd add a +1 to the procfs approach.
procfs
This is a necessary optimization in Intel machines with KPTI patches applied, since making a million close() calls is very expensive. Is there something holding the PR from being merged?
close()
This needs to catch a specific exception type, not just any exception. Also, when the exception is caught, it's just discarded; instead, it should be handled or not caught at all.
The code doesn't make clear why this is being checked. Maybe a comment inside the if block to say what condition has been detected?
if
Why this name? Can you choose something that makes clearer the meaning of this value?
It's not clear what the purpose of this specific condition is. Can you write a comment inside the if block, to say what the meaning of this condition is?
Thank you, this correction is now incorporated into master.
master
I've written some comments on specific code.
To be considered, this will at least need full unit test coverage; it's not clear what should or should not make all these branches happen.
Metadata Update from @bignose: - Pull-request tagged with: moreinfo
If you have a working /proc/{pid}/fd then why even bother with building ranges? Use os.listdir to build a list of open file descriptors, less those passed as an exclusion list. Then just close those, instead of using ranges of possible descriptors. e.g.
/proc/{pid}/fd
os.listdir
# MAXFD, get_maximum_file_descriptors(), _get_candidate_file_descriptors(), # FileDescriptorRange, _get_candidate_file_descriptor_ranges(), # _close_file_descriptor_ranges() no longer required def close_all_open_files(exclude=None): """ Close all open file descriptors. :param exclude: Collection of file descriptors to skip when closing files. :return: ``None``. Closes every file descriptor (if open) of this process. If specified, `exclude` is a set of file descriptors to *not* close. Requires the /proc/{PID}/fd virtual filesystem. """ try: pid = os.getpid() path = "/proc/{}/fd".format(pid) for x in os.listdir(path): x = int(x) if exclude is None or x not in exclude: close_file_descriptor_if_open(x) except OSError as exc: error = DaemonOSEnvironmentError( "Failed to list open file descriptors ({exc})".format( exc=exc)) raise error
In the absence of the specific information requested, I'm closing this merge request.
Please feel free to describe a new merge request if it's still needed.
Pull-Request has been closed by bignose