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

Avoid leaking fds in test suite and NewMapWithOptions() #725

Merged
merged 2 commits into from
Jul 1, 2022

Conversation

ti-mo
Copy link
Collaborator

@ti-mo ti-mo commented Jul 1, 2022

Since omitting Map and Program.Close() is generally considered an error,
clean up all cases in the test suite.

These were found by adding runtime.GC() at the end of a TestMain()
in all packages in the repository.

The plan is to follow up with some instrumentation that allows the caller
to get notified when the sys.FD finalizer successfully closes a file
descriptor during a GC round.
If a Map fails to be populated or frozen during creation,
we would leak a fd for the runtime to collect. This patch ensures
the fd is closed before returning from NewMapWithOptions.

ti-mo added 2 commits July 1, 2022 16:50
If a Map fails to be populated or frozen during creation,
we would leak a fd for the runtime to collect. This patch ensures
the fd is closed before returning from NewMapWithOptions.

Signed-off-by: Timo Beckers <timo@isovalent.com>
Since omitting Map and Program.Close() is generally considered an error,
clean up all cases in the test suite.

These were found by adding runtime.GC() at the end of a TestMain()
in all packages in the repository.

The plan is to follow up with some instrumentation that allows the caller
to get notified when the sys.FD finalizer successfully closes a file
descriptor during a GC round.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo requested a review from lmb July 1, 2022 14:52
@ti-mo ti-mo merged commit eb74eec into cilium:master Jul 1, 2022
@ti-mo ti-mo deleted the tb/leaking-fds branch July 1, 2022 15:00
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.

1 participant