mount: Wrap yields in context managers with try-finallyrichardmaw/fix-unit-test-hang-on-crash

The terminator context will only clean up on a signal,
so if another exception causes context manager cleanup
then unmount won't be called unless it's part of another context manager
or is wrapped in a try block's except or finally.

Everywhere else's unmounts are handled by delegating to another context manager
but these were what needed to be fixed.

The change in buildstream/_fuse/mount.py would cause lockups
since the build worker process still having a subprocess
blocked its termination from completing
which in turn caused the pipeline manager process to block indefinitely on it.

2 files changed, 19 insertions, 17 deletions

diff --git a/buildstream/_fuse/mount.py b/buildstream/_fuse/mount.py
index 8220a8d07..3848ad305 100644
--- a/buildstream/_fuse/mount.py
+++ b/buildstream/_fuse/mount.py
@@ -143,12 +143,11 @@ class Mount():
     @contextmanager
     def mounted(self, mountpoint):
 
-        def kill_proc():
-            self.unmount()
-
-        with _signals.terminator(kill_proc):
-            self.mount(mountpoint)
-            yield
+        self.mount(mountpoint)
+        try:
+            with _signals.terminator(self.unmount):
+                yield
+        finally:
             self.unmount()
 
     ################################################
diff --git a/buildstream/sandbox/_mounter.py b/buildstream/sandbox/_mounter.py
index 3f8299392..c039b31df 100644
--- a/buildstream/sandbox/_mounter.py
+++ b/buildstream/sandbox/_mounter.py
@@ -98,10 +98,12 @@ class Mounter(object):
 
         options = ','.join([key for key, val in kwargs.items() if val])
 
-        with _signals.terminator(kill_proc):
-            yield cls._mount(dest, src, mount_type, stdout=stdout, stderr=stderr, options=options)
-
-        cls._umount(dest, stdout, stderr)
+        path = cls._mount(dest, src, mount_type, stdout=stdout, stderr=stderr, options=options)
+        try:
+            with _signals.terminator(kill_proc):
+                yield path
+        finally:
+            cls._umount(dest, stdout, stderr)
 
     # bind_mount()
     #
@@ -136,10 +138,11 @@ class Mounter(object):
 
         path = cls._mount(dest, src, None, stdout, stderr, options)
 
-        with _signals.terminator(kill_proc):
-            # Make the rbind a slave to avoid unmounting vital devices in
-            # /proc
-            cls._mount(dest, flags=['--make-rslave'])
-            yield path
-
-        cls._umount(dest, stdout, stderr)
+        try:
+            with _signals.terminator(kill_proc):
+                # Make the rbind a slave to avoid unmounting vital devices in
+                # /proc
+                cls._mount(dest, flags=['--make-rslave'])
+                yield path
+        finally:
+            cls._umount(dest, stdout, stderr)