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

Fix LinuxEvents destructor to close all fd #15

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

Conversation

Jens-H
Copy link

@Jens-H Jens-H commented Nov 28, 2020

Here would be how I solved running out of fds.

@lemire
Copy link
Member

lemire commented Nov 28, 2020

Thanks for the PR. Can you explain this code?

@Jens-H
Copy link
Author

Jens-H commented Nov 28, 2020

Certainly. It takes the last opened file descriptor (fd) assigned in the for-loop lines 41-51. Then it closes this fd and the next x-1 smaller fd, where x is the number of perf events (stored in num_events) for the LinuxEvents instance. Obviously this is a hack, as the really opened fd are not used, but guessed.

@lemire lemire linked an issue Nov 29, 2020 that may be closed by this pull request
Copy link
Contributor

@thomasmueller thomasmueller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about keeping the list of file descriptors in a std::vector?

@Jens-H
Copy link
Author

Jens-H commented Dec 1, 2020

What about keeping the list of file descriptors in a std::vector?

Most probably a cleaner and more general solution.

I should add that I am not sure how the start/end method is working with just the last fd. This is why I decided to just do a fix in the destructor and leave everything else as it is.

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.

Not all fd are closed in destructor
3 participants