Windows: handle remains open after watch-del command

[derrickstolee]

I'm using Git's fsmonitor tool with Watchman to speed up performance when there are many files. However, after using the repo, I'm unable to delete it without closing the watchman process (on Windows). There must be an open handle somewhere still.

I'm using the watch-del command to remove the watch, but the handle still is not closed.

$ watchman watch-list
{
    "version": "4.9.4",
    "roots": [
        "C:/_git/ForTests/src"
    ]
}

$ rm -rf ForTests
rm: cannot remove 'ForTests/src': Device or resource busy

$ watchman watch-del "C:/_git/ForTests/src"

$ watchman watch-list
{
    "version": "4.9.4",
    "roots": []
}

$ rm -rf ForTests
rm: cannot remove 'ForTests/src': Device or resource busy

$ handle.exe | grep -B 1 ForTests
cmd.exe pid: 24192 DESKTOP-LE3SIE3\stole
   40: File  (RW-)   C:\_git\ForTests\src
--
watchman.exe pid: 22960 DESKTOP-LE3SIE3\stole
   44: File  (RW-)   C:\_git\ForTests\src

(The "cmd.exe" process reported by handle.exe is the background runner that is running watchman.exe as a child.)

I'm struggling to read the code and connect the dots (especially because I've failed to build it locally). I see that FileDescriptor has a destructor that will clear the handle, and that should be part of the destructor for WinDirHandle (which extends watchman_dir_handle). What I don't see is any connection between the code in watchlist.cpp and those types.

If it helps, it is important to note that a query must be asked of watchman in order to trigger this failure. This Python test may demonstrate the bug:

@WatchmanTestCase.expand_matrix
class TestHandles(WatchmanTestCase.WatchmanTestCase):
    def checkOSApplicability(self):
        if sys.platform != "win32":
            self.skipTest("N/A unless Windows")

    def test_handles(self):

        root = self.mkdtemp()
        self.watchmanCommand("watch", root)
        self.touchRelative(root, "file")
        self.watchmanCommand("watch-del", root)
        os.rmdir(root)
        self.assertFalse(os.path.isdir("root"))

I'm happy to commit time to fix this issue, but I'll need a little guidance for where to look. My initial inspection has led me in circles so I figured I would write this issue while everything is fresh.

[wez]

Thanks for reporting this and digging in! I'm technically on PTO and away from work this week so can't look too closely until I get back, but since you'd started poking around, I thought you'd be ok with some pointers in the mean time!

I'm confident that FileDescriptor is generally managing resources correctly as we'd likely have seen issues on the other platforms and see more than just a single handle in the handle.exe output you shared.

That makes me wonder if something at a higher level is holding on to things. https://github.com/facebook/watchman/blob/master/watcher/win32.cpp is where we do the windows specific watching stuff; I wonder if there is a thread that is stuck holding on to a shared_ptr that is transitively keeping the dir_handle alive? this code captures a reference to the watcher and attempts to lock something during its shutdown and is something that is worth checking first; getting a stack trace from all threads may be helpful!

When it comes to building, it is a bit awkward to grab the deps piecemeal, but we do have a helper script that will download and build all the relevant deps as well as watchman itself; you just need a recent version of visual studio and python to be installed and it will take care of the rest; it will likely take 20-30 minutes to build the dependencies on the first build run, depending on how powerful your system is.

python build/fbcode_builder/getdeps.py build --src-dir=. watchman

You can run the test suite locally after you've built things using this:

python build/fbcode_builder/getdeps.py test --src-dir=. watchman

[derrickstolee]

Thanks for the pointer! I'll give that a shot. I'm about to be out until Dec 9 myself, so I'll come back to this later.

For now, I found that I'm able to hack this approach using watchman shutdown-server, which unblocks the deletion but also interrupts any current queries. It gets me unblocked for now, but I'll definitely come back to this soon.

[wez]

I've been poking at this today and haven't been able to reproduce this issue :-/
Is it possible that one of the threads that owns a handle is blocked in the scenario
that triggers this for you?
Could you capture and share a stack trace from all of the threads in the process?

[wez]

I'm going to consider this closed, but will be happy to re-open if we can get this to reproduce with a more recent build.