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

Fixes log reopening. #1

Merged
merged 2 commits into from
Mar 20, 2017
Merged

Fixes log reopening. #1

merged 2 commits into from
Mar 20, 2017

Conversation

vladlosev
Copy link

This fixes a situation when logs are rotated via movement.

  1. A log is rotated.
  2. The library sees that, closes the watch on the file, and sets up a directory watch for the file to appear.
  3. New log file is created.
  4. The library sees the file created, and removes the directory watch
  5. The library creates a watch on the file itself now that the file is present.

In the current code, (4) cleans up the done channel but then removes the directory watch and exits without instructing the InotifyTracker::run to complete the cleanup via the removeWatch method. This results in the event channel for the file not being cleaned up. It's not a problem in itself, but then in (5), addWatch uses the presence of the event channel to decide whether initialize both events and done channel. As the event channel is still present, the done channel is not initialized. Subsequently, absence of the done channel prevents sendEvents from forwarding fsnotify events to clients.

This change makes a separate check for presence of the of the done channel in order to initialize it. The changes in the test introduce stricter checks: without them the test hangs and is subsequently terminated as lines are not being delivered to the reader.

@vladlosev
Copy link
Author

@negator would like your 👀 on this. Please also loop in anyone else who can contribute a good review. Once this looks good, I will send another PR against upstream.

@negator
Copy link
Member

negator commented Mar 20, 2017

lgtm

@vladlosev vladlosev merged commit 519b438 into master Mar 20, 2017
@vladlosev vladlosev deleted the fix-log-reopen-with-inotify branch March 20, 2017 18:42
@vladlosev vladlosev restored the fix-log-reopen-with-inotify branch March 20, 2017 18:51
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