Fix data race and deadlock in file_integrity module

[adriansr]

This PR fixes two issues with auditbeat's file_integrity module:

• Concurrent map write in [Auditbeat] TestIncludedExcludedFiles fails with race condition #8009
  Auditbeat's file_integrity module had a data race in its recursive watcher implementation for Linux. Was caused by file watches being installed from different goroutines.
  This patch fixes the problem by moving watch installation to the same goroutine that processes inotify's events, guaranteeing that watches are installed in order, which is necessary for consistence.

• deadlock when file integrity monitor is started
  A deadlock is possible in auditbeat's file_integrity module under Windows: When enough events arrive while watches are being installed, the event channel can fill causing the installation of a watch to block. This patch makes sure that events are received while watches are being installed, and at the same time ensures that no event is lost.

[adriansr]

There seems to be a problem I didn't catch in local testing done

[andrewkroh]

When the writer is done with queueC it should be closed.

[adriansr]

done

[andrewkroh]

Good idea.

[cwurm]

Regarding the deadlock fix: I wonder if it would be possible to do the watch installation in a goroutine to prevent the deadlock?

Something like this in eventreader_fsnotify.go:

func (r *reader) Start(done <-chan struct{}) (<-chan Event, error) {
  ...
  go r.consumeEvents(done)
  go r.installWatches()
  ...
  return r.eventC, nil
}

My thinking is that that way, Start would return right away, allowing init/run in metricset.go to start consuming events, preventing a block.

I might be missing something.

[adriansr]

@cwurm, that's how I implemented it at first. However, it introduces an inconsistency that will trigger failures in tests, as the test may be performing an action right after Start but before a watch is installed, causing an expected event to be lost.

[cwurm]

@adriansr Ah, shame. LGTM then.

Quick question on the concurrent map write: Would synchronizing read/write methods of FileTree have worked as well? Do you think that's something we should do at some point?

[adriansr]

Yes @cwurm, that would have fixed the problem too. I felt it made more sense to confine all the accesses in the same place.