Fixes issue where Unix crashes when inotify is full #6637
Conversation
Previously, the program was crashing for Unix users when inotify had reached its max file limit. Since the only way to fix this seems to be to close applications or to edit privileged system files, the proposed solution is to notify the user of the issue and give them the option of continuing usage of the application. fixes #6073
Previously, the program was crashing for Unix users when inotify had reached its max file limit. Since the only way to fix this seems to be to close applications or to edit privileged system files, the proposed solution is to notify the user of the issue and give them the option of continuing usage of the application. fixes #6073
There was a problem hiding this comment.
Thank you for the PR. Could you try to reformat the code according to our style rules (otherwise CheckStyle will fail:
Could you create a FAQ entry to https://github.com/JabRef/user-documentation/blob/master/en/faqgeneral.md? (Taking the links from #6073)
| @@ -29,11 +31,14 @@ | |||
|
|
|||
| private final Multimap<Path, FileUpdateListener> listeners = ArrayListMultimap.create(20, 4); | |||
| private WatchService watcher; | |||
| private Optional<JabRefException> filesystemMonitorFailure; | |||
There was a problem hiding this comment.
I think, it's OK to store the exception (for future work).
There was a problem hiding this comment.
What do you mean? I am not sure what the code change is that you are requesting.
There was a problem hiding this comment.
This was more a comment to a second reviewer.
Style edits according to JabRef preferences.
ghost
mentioned this pull request
ghost
requested a review
from
There was a problem hiding this comment.
Thank you for the quick update. Still have some comments about the localization.
@calixtus Maybe we need to have a discussion about the archicture here. I did not look deep into the code about the control flow of the Exception not being thrown any more, but at some later point in time checked for an error.
| @@ -9,12 +8,17 @@ | |||
| */ | |||
| public class DummyFileUpdateMonitor implements FileUpdateMonitor { | |||
| @Override | |||
| public void addListenerForFile(Path file, FileUpdateListener listener) throws IOException { | |||
| public void addListenerForFile(Path file, FileUpdateListener listener) { | |||
There was a problem hiding this comment.
The exception was removed here, but not in the interface. I think, it should also be removed here (and the JavaDoc be adapted).
With the current implementation, it does not throw an exception if the file does not exist.
|
Since this is an important fix, it should not be stalled. @cttillman We hope that you'll include other improvements in JabRef, so we want to share knowledge about the coding ideas in JabRef. The localization is a difficult thing here. Would you mind to fix the localization isse?
|
Added #6073 error message to localizaiton file.
Siedlerchr
added
the
status: ready-for-review
Previously, the program was crashing for Unix users when inotify had reached its max file limit. Since the only way to fix this seems to be to close applications or to edit privileged system files, the proposed solution is to notify the user of the issue and give them the option of continuing usage of the application. Fixes #6073