Skip to content

[release/0.8] don't crash in cache refresh/update with nil fsnotify watcher. #255

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

Conversation

klihub
Copy link
Contributor

@klihub klihub commented Feb 24, 2025

Cherry picked #254 to release/0.8 branch.

Don't crash in update() if we fail to create an fsnotify watch.
This can happen if we have too many open files. In this case we
now record a failure for all configured spec directories and in
update we always trigger a refresh. If the process if ever able
to create new file descriptors the cache becomes functional but
in a 'always implicitly fully refreshed' mode instead of auto-
refreshed.

It's not entirely clear what is the best option to deal with a
failed watch creation. Being out of file descriptors typically
results in a cascading chain of errors which the process does
not usually survive.

This fix aims for minimal footprint. On failed watch creation
it does not render the cache fully unusable. If the process is
ever able to create new file descriptors again the cache also
becomes functional, but instead of autorefreshed mode it will
be in an 'always implicitly fully refreshed' mode.

Signed-off-by: Krisztian Litkey <[email protected]>
Test that the cache indeed recovers functionally if the process
ever recovers from not being able to create new file descriptors.

Signed-off-by: Krisztian Litkey <[email protected]>
@klihub klihub requested review from elezar and bart0sh February 24, 2025 10:30
@bart0sh bart0sh merged commit d1a2c36 into cncf-tags:release/0.8 Feb 24, 2025
6 checks passed
@klihub klihub deleted the fixes/release-0.8/refresh-sigsegv-with-nil-watcher branch February 24, 2025 12:09
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.

2 participants