Skip to content

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

Merged
bart0sh merged 3 commits intocncf-tags:release/0.8from
klihub:fixes/release-0.8/refresh-sigsegv-with-nil-watcher
Feb 24, 2025
Merged

[release/0.8] don't crash in cache refresh/update with nil fsnotify watcher.#255
bart0sh merged 3 commits intocncf-tags:release/0.8from
klihub:fixes/release-0.8/refresh-sigsegv-with-nil-watcher

Conversation

@klihub
Copy link
Contributor

@klihub klihub commented Feb 24, 2025

Cherry picked #254 to release/0.8 branch.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
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 <krisztian.litkey@intel.com>
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 <krisztian.litkey@intel.com>
@klihub klihub requested review from bart0sh and elezar 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