avoid invariant violations due to previously crashed processes

Summary:
Prior to this diff, the following sequence of events caused an invariant violation:

1. `hh_server` is not running.
2. Some other application A crashes while asserting a state X, leaving a spurious X.notify file
3. `hh_server` is started, and correctly considers state X as not being asserted
4. Application A is started again and asserts X again. This causes it to delete the X.notify file first, which triggers a `StateLeave(X)` event followed by a `StateEnter(X)`.
5. `hh_server` receives `StateLeave(X)` violating the invariant that we must note leave a state that's not asserted.

Note that this can only happen if A crashed *before* hh was started. We already handle the case correctly when an application asserting a state crashes while `hh_server` is running.

This diff fixes this issue: As of D91942985, `which_states_asserted` cleans up all spurious X.notify files for us during server initialization. The only thing this diff does is ensuring that we ask Eden for the initial journal position during initialization *after* calling `which_states_asserted`, so that we skip over the `StateLeave` events due to the cleanup. Note that this also means that our code in `Edenfs_watcher` continues not to interact with .notify files.

As a drive-by change, I'm also switching us from using `get_asserted_states` to `which_states_asserted` for getting the initially asserted states during server start. The only difference between the two is that the latter is supposed to be the public API and does the filtering of state names for us.

All the changes in this diff only affect behaviour if `edenfs_file_watcher_state_tracking` is enabled, which is disabled for everyone at the moment.

Reviewed By: Alramech

Differential Revision: D92404246

fbshipit-source-id: 88f21edbc46737ebc173e0ce7fec8d1c4b4fe19b

Author: frank-emrich

hphp/hack/test/integration/test_edenfs_file_watcher.py

@@ -454,16 +454,17 @@ def createIgnoredFiles(cls) -> None:
 
     def assertStateForSeconds(
         self, state_name: str, duration_secs: int
-    ) -> Tuple[Callable[[], None], Callable[[], bool]]:
+    ) -> Tuple[Callable[[], None], Callable[[], bool], Callable[[], None]]:
         """Asserts the given state for the given number of seconds with Eden.
 
         This function returns as soon as the state is asserted.
         The state then remains asserted in the background.
 
-        Returns to thunks:
+        Returns three thunks:
         - Calling the first one waits until the state is de-asserted
         - Calling the second returns a boolean indicating whether the state is
            still asserted.
+        - Calling the third kills the asserter process with SIGKILL.
         """
 
         eden_state_asserter_path = os.getenv("HH_EDEN_TEST_STATE_ASSERTER", None)
@@ -496,7 +497,10 @@ def wait_and_check() -> None:
         def is_still_asserted() -> bool:
             return proc.poll() is None
 
-        return (wait_and_check, is_still_asserted)
+        def kill_asserter() -> None:
+            proc.kill()
+
+        return (wait_and_check, is_still_asserted, kill_asserter)
 
 
 class EdenfsWatcherTests(common_tests.CommonTests):
@@ -1155,7 +1159,7 @@ def test_deferral1(self) -> None:
 
         self.test_driver.createNonHackFile("file1.php")
 
-        wait_for0, is_asserted0 = self.test_driver.assertStateForSeconds(state0, 10)
+        wait_for0, is_asserted0, _ = self.test_driver.assertStateForSeconds(state0, 10)
 
         self.test_driver.createNonHackFile("file2.php")
 
@@ -1200,7 +1204,7 @@ def run_instant_deassertion_test(
         # 30 iterations keeps this test at around 1.5 minutes
         for iterations in range(1, 30):
             for _ in range(iterations):
-                wait_thunk, _ = self.test_driver.assertStateForSeconds(state, 0)
+                wait_thunk, _, _ = self.test_driver.assertStateForSeconds(state, 0)
                 if wait_for_deassert:
                     wait_thunk()
 
@@ -1239,7 +1243,7 @@ def test_deferral6(self) -> None:
 
         self.test_driver.createNonHackFile("file0.php")
 
-        wait_for0, _ = self.test_driver.assertStateForSeconds(state0, 20)
+        wait_for0, _, _ = self.test_driver.assertStateForSeconds(state0, 20)
 
         self.test_driver.createNonHackFile("file1.php")
 
@@ -1250,7 +1254,7 @@ def test_deferral6(self) -> None:
         )
 
         # will be deasserted while state0 is still asserted
-        wait_for1, _ = self.test_driver.assertStateForSeconds(state1, 10)
+        wait_for1, _, _ = self.test_driver.assertStateForSeconds(state1, 10)
 
         self.test_driver.createNonHackFile("file2.php")
 
@@ -1304,7 +1308,7 @@ def test_deferral7(self) -> None:
 
         self.test_driver.createNonHackFile("file0.php")
 
-        wait_for0, _ = self.test_driver.assertStateForSeconds(state0, 20)
+        wait_for0, _, _ = self.test_driver.assertStateForSeconds(state0, 20)
 
         self.test_driver.createNonHackFile("file1.php")
 
@@ -1316,7 +1320,7 @@ def test_deferral7(self) -> None:
         )
 
         # will be deasserted while state0 is still asserted
-        wait_for1, _ = self.test_driver.assertStateForSeconds(state1, 10)
+        wait_for1, _, _ = self.test_driver.assertStateForSeconds(state1, 10)
 
         self.test_driver.createNonHackFile("file2.php")
 
@@ -1350,7 +1354,7 @@ def test_deferral8(self) -> None:
         )
 
         state = TEST_STATE_0
-        _, is_asserted = self.test_driver.assertStateForSeconds(state, 20)
+        _, is_asserted, _ = self.test_driver.assertStateForSeconds(state, 20)
 
         # Note that we asserted before server startup
         self.test_driver.start_hh_server()
@@ -1377,7 +1381,7 @@ def test_deferral9(self) -> None:
         self.test_driver.start_hh_server()
 
         # Assert a state that is NOT in the tracked_states list
-        _, is_still_asserted = self.test_driver.assertStateForSeconds(
+        _, is_still_asserted, _ = self.test_driver.assertStateForSeconds(
             "untracked-state", 30
         )
 
@@ -1394,6 +1398,64 @@ def test_deferral9(self) -> None:
         # ... and we should also not have waited until deferral.
         self.assertTrue(is_still_asserted())
 
+    def test_deferral10(self) -> None:
+        """Test asserting application crashing while hh is running"""
+        config = self.test_driver.getConfig()
+        config.state_tracking = True
+        config.write_hhconf(
+            self.test_driver.watchman_instance.getUnixSockPath(),
+            self.test_driver.repo_dir,
+        )
+
+        self.test_driver.start_hh_server()
+        self.test_driver.check_cmd(["No errors!"])
+
+        for i, kill_after_secs in enumerate([0, 0.1, 0.5, 1]):
+            # Assert state for a long duration (we'll kill it before it finishes)
+            _, _, kill_asserter = self.test_driver.assertStateForSeconds(
+                TEST_STATE_0, 60
+            )
+
+            time.sleep(kill_after_secs)
+
+            # SIGKILL the asserter while it's asserting TEST_STATE_0
+            kill_asserter()
+
+            # The asserter is dead, we should not be deferring anymore
+            file = f"file{i}.php"
+            self.test_driver.createNonHackFile(file)
+            self.test_driver.check_cmd(
+                [
+                    f"ERROR: {{root}}{file}:1:1,1: A .php file must begin with `<?hh`. (Parsing[1002])",
+                ]
+            )
+            os.remove(os.path.join(self.test_driver.repo_dir, file))
+
+    def test_deferral11(self) -> None:
+        """Regression test for when an application asserting a state crashed before starting the server."""
+        config = self.test_driver.getConfig()
+        config.state_tracking = True
+        config.write_hhconf(
+            self.test_driver.watchman_instance.getUnixSockPath(),
+            self.test_driver.repo_dir,
+        )
+
+        # Assert state for a long duration (we'll kill it before it finishes)
+        _, _, kill_asserter = self.test_driver.assertStateForSeconds(TEST_STATE_0, 60)
+
+        # SIGKILL the asserter while it's asserting TEST_STATE_0
+        kill_asserter()
+
+        self.test_driver.start_hh_server()
+        self.test_driver.check_cmd(["No errors!"])
+
+        # Let's assert the state again
+        wait_for_deassert, _, _ = self.test_driver.assertStateForSeconds(
+            TEST_STATE_0, 1
+        )
+        wait_for_deassert()
+        self.test_driver.check_cmd(["No errors!"])
+
 
 class EdenfsWatcherNonMountPointRepoTests(EdenfsWatcherTests):
     """Runs the same tests as EdenfsWatcherTests, but with a testing repo that's not the mount point of the Eden mount.