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

linux driver can abort the enclosing process #509

Closed
ColinDKelley opened this issue Nov 6, 2020 · 4 comments · Fixed by #545
Closed

linux driver can abort the enclosing process #509

ColinDKelley opened this issue Nov 6, 2020 · 4 comments · Fixed by #545

Comments

@ColinDKelley
Copy link
Collaborator

ColinDKelley commented Nov 6, 2020

Current State

If :INotify::Notifier.new (or #watch) were to raise Errno::ENOSPC, the linux driver would abort the containing process:

      def _configure(directory, &callback)
        require 'rb-inotify'
        @worker ||= ::INotify::Notifier.new
        @worker.watch(directory.to_s, *options.events, &callback)
      rescue Errno::ENOSPC
        abort(INOTIFY_LIMIT_MESSAGE)
      end

Calling abort from a gem is never appropriate.

Desired State

Errno::ENOSPC should be wrapped by raising a named exception (with ENOSPC as the cause). That will raise out of the public interface start. The calling code in the containing process can then take the appropriate action, by rescuing that specific exception or a catch-all base class.

Steps to Reproduce

I didn't try, but you'd need to run out the count of inotify file descriptors. Then call listener.start. That would cause the containing process to abruptly abort.

@ColinDKelley
Copy link
Collaborator Author

@ioquatix Do you agree that this should be fixed? I've never seen it happen in production...but we'd be appalled if it did. It could cause a total outage of a company's service! Yikes. This feels like a "ticking time bomb".

@ColinDKelley ColinDKelley changed the title linux driver will abort the enclosing process linux driver can abort the enclosing process Nov 6, 2020
@ioquatix
Copy link
Member

ioquatix commented Nov 6, 2020

Why was it added in the first place?

@ColinDKelley
Copy link
Collaborator Author

That line hasn't been touched in 7 years: f9e8220

@ColinDKelley
Copy link
Collaborator Author

But it appears to have first been added 8.5 years ago: 5e3053d

ColinDKelley added a commit to Invoca/listen that referenced this issue Aug 15, 2021
ColinDKelley added a commit to Invoca/listen that referenced this issue Aug 15, 2021
ColinDKelley added a commit to Invoca/listen that referenced this issue Aug 15, 2021
ColinDKelley added a commit to Invoca/listen that referenced this issue Aug 18, 2021
ColinDKelley added a commit to Invoca/listen that referenced this issue Aug 18, 2021
ColinDKelley added a commit to Invoca/listen that referenced this issue Aug 18, 2021
ColinDKelley added a commit to Invoca/listen that referenced this issue Aug 18, 2021
ColinDKelley added a commit to Invoca/listen that referenced this issue Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants