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

Test regressions introduced in #453 #459

Open
ChrisCummins opened this issue Oct 11, 2021 · 5 comments
Open

Test regressions introduced in #453 #459

ChrisCummins opened this issue Oct 11, 2021 · 5 comments
Assignees

Comments

@ChrisCummins
Copy link
Contributor

🐛 Bug

This issue tracks progress towards addressing the errors of the type:

ResourceWarning: unclosed <socket.socket fd=13, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0>

that were introduced in #453.

ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Oct 11, 2021
@ChrisCummins ChrisCummins self-assigned this Oct 11, 2021
ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Oct 11, 2021
ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Oct 11, 2021
@ChrisCummins ChrisCummins changed the title ResourceWarning: unclosed <socket.socket fd=?, family=AddressFamily.?, ...> Test regressions introduced in #453 Oct 11, 2021
ChrisCummins added a commit that referenced this issue Nov 8, 2021
ChrisCummins added a commit that referenced this issue Nov 8, 2021
@sogartar
Copy link

I also encountered this on my Ubuntu 20.04 environment.
Here is my tests log log-build-2021-12-15T01-06-01Z-failed-tests.txt.

I think it is caused due to difference in pip dependencies.
Here is the list of my pip packages pip-pakcages.txt.

My intuition is that at a newer versions pytest checks for resources that are not freed at the end of tests.
This answer also points to a similar conclusion https://stackoverflow.com/a/54612520/324204
Probably a service is not being closed correctly or the service itself does not close the socket when destroyed.

@ChrisCummins
Copy link
Contributor Author

Yep, that looks like the same kind of errors. Thanks for sharing your logs and package lists.

My intuition is that at a newer versions pytest checks for resources that are not freed at the end of tests.

I think that's right. I first noticed the error after bumping my pytest version.

Probably a service is not being closed correctly or the service itself does not close the socket when destroyed.

That's what I thought too, but I haven't been find the time to audit all of the uses of sockets in the codebase to narrow down the cause. As a result I've just been marking failing tests as xfail (which is terrible form, but there's only so many hours in the day 😄), though I think there is still some residual flakiness in the CI caused by this issue.

Cheers,
Chris

@ChrisCummins
Copy link
Contributor Author

BTW I wasn't able to narrow down whether the unclosed resource is from a subprocess (#326), or an open logfile / other resource.

@sogartar
Copy link

One strategy to debug this is to log the stack and file descriptor when file descriptors are opened. Then you would know which file descriptor is left open.
This implies adding this logging to all places in the code that create sockets.

@ChrisCummins
Copy link
Contributor Author

That's a good idea :) I'll try and carve out some time in mid January to take a look.

Cheers,
Chris

ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Dec 22, 2021
These should be addressed by the new Popen() usage.

Issue facebookresearch#459.
ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Feb 18, 2022
ChrisCummins added a commit to ChrisCummins/CompilerGym that referenced this issue Mar 6, 2022
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

No branches or pull requests

2 participants