gitLsFiles deadlocks on largish repos

[Bodigrim]

https://github.com/NorfairKing/feedback/blob/d7fdda89d45d76f4ea6ddc8e73cbe1d13b763478/feedback/src/Feedback/Loop/Filter.hs#L68-L75

This code is problematic: it creates a pipe for stdout, but does not read from it until the process exits. This works at small scale, as long as the full output fits into pipe's buffer (typically up to 64K), but otherwise deadlocks: the process cannot exit, because it has not finished with its output, but the pipe is full and no one reads from it.

I originally (https://github.com/NorfairKing/feedback/issues/5#issuecomment-1137923950) thought that this is a Windows-specific issue, but it is not. One can reproduce it by running feedback inside a git repo, where git ls-files takes more than 64K. E. g., GHC repo is big enough.

filesFromFindArgs has the same defect.

---

FWIW I'm not quite convinced that the usage of conduit is fully justified: one cannot lazily return Set FilePath, so nothing is actually streamed and all data is loaded into memory anyway. A straightforward readProcess should work to the same effect.

[NorfairKing]

Jup I think I've noticed this once before. What do you think, should we use per-event checking instead?

[Bodigrim]

I don't really have a good intuition in this area (and the investigation above is mostly courtesy of @sergv). My hypothesis is that you are unlikely to watch a folder, which is so massive that you cannot hold all filenames in the memory at once, so there is no real need for streaming and one can just read stdout in full, split it into lines, convert to Set FilePath and later attach listeners in the same way as currently.

[NorfairKing]

OH! I just understood your original comment. Yes that's a bug of course. Going on my list! I'll fix it before considering switching to per-event checking, I think.

[NorfairKing]

Fixed in 3928be61d28d480d043eb01394c8e55478ab969e, but it's quite slow for big repos (I tried it on nixpkgs)...

[Bodigrim]

Thanks!