libct/test: Disable GC on test run to catch leaking fds#5243
libct/test: Disable GC on test run to catch leaking fds#5243cyphar merged 1 commit intoopencontainers:mainfrom
Conversation
rata
force-pushed
the
fix-flaky-TestFdLeaks
branch
from
4834716 to
3e6e7a1
Compare
| } | ||
| } | ||
|
|
||
| dst, err := linux.Readlinkat(procSelfFd, fd1) |
There was a problem hiding this comment.
- Why not add error handing to the previous Readlinkat?
- Shouldn't we only ignore ENOENT and report other errors?
There was a problem hiding this comment.
- Grr, I screw it up, I had several changes and clearly I applied the diff incorrectly. Sorry, will fix now
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
Yeah I guess the only issue with the test is missing error checking from readlinkat.
I'd rather use a step-by-step approach at fixing this, starting with one simple fix (e.g. error checking and ignoring ENOENT) and seeing it if helps.
|
Another idea: we can also try to call runtime.GC so that any fds that are to be closed by finalizers etc will be closed. |
|
@kolyshkin heh, it's weird, but I thought about that too. Because it will close fds that we are leaking too. I think we can disable the GC for the test and that should do it. But honestly, at this point I doubt if we should test in a completely different way. Let's see what comes out of this GC idea... |
rata
force-pushed
the
fix-flaky-TestFdLeaks
branch
from
3e6e7a1 to
7c089d0
Compare
rata
changed the title
|
@kolyshkin reworked it to use the GC instead. Let me know what you think. I think ignoring readlink is weak, as we could be leaking fds and the GC closing them for us. But now with GC disabled, I guess the original test shouldn't be flaky and still reliable |
|
OK, the first fedora run (with the new patch disabling GC) failed with an unrelated test (#5013), and the second run timed out. |
This test is racy for a long time now. All the logs I could find in CI seem to be dangling symlinks, like the test shows "23 -> ". This means the fd was closed before we did the call to readlink(). Let's try to disable the GC. This should get rid of the "fds are getting closed before we read them" part. Updates: opencontainers#4297 Signed-off-by: Rodrigo Campos <rodrigo@amutable.com>
kolyshkin
force-pushed
the
fix-flaky-TestFdLeaks
branch
from
7c089d0 to
748af2e
Compare
cyphar
left a comment
cyphar
left a comment
There was a problem hiding this comment.
I like the idea of disabling GC, this probably makes our tests for leaks even more strict while also reducing flakiness. Thanks!
3 participants
libct/test: Disable GC on test run to catch leaking fds
This test is racy for a long time now. All the logs I could find in CI
seem to be dangling symlinks, like the test shows "23 -> ". This means
the fd was closed before we did the call to readlink().
Let's try to disable the GC. This should get rid of the "fds are getting
closed before we read them" part.
Updates: #4297