Improve expection handling and make methods private

ospd_openvas/lock.py

@@ -35,7 +35,7 @@ def __init__(self, path: Path):
     def has_lock(self) -> bool:
         return self._has_lock
 
-    def acquire_lock(self) -> "LockFile":
+    def _acquire_lock(self) -> "LockFile":
         """ Acquite a lock by creating a lock file.
         """
         if self.has_lock():
@@ -47,12 +47,17 @@ def acquire_lock(self) -> "LockFile":
             parent_dir.mkdir(parents=True, exist_ok=True)
 
             self._fd = self._lock_file_path.open('w')
-        except PermissionError as e:
+        except Exception as e:  # pylint: disable=broad-except
             logger.error(
                 "Failed to create lock file %s. %s",
                 str(self._lock_file_path),
                 e,
             )
+            try:
+                self._fd.close()
+                self._fd = None
+            except (AttributeError, TypeError):
+                pass
             return self
 
         # Try to adquire the lock.
@@ -64,31 +69,36 @@ def acquire_lock(self) -> "LockFile":
             logger.error(
                 "Failed to lock the file %s. %s", str(self._lock_file_path), e,
             )
-            self._fd.close()
+            try:
+                self._fd.close()
+                self._fd = None
+            except (AttributeError, TypeError):
+                pass
 
         return self
 
     def wait_for_lock(self):
         while not self.has_lock():
-            self.acquire_lock()
+            self._acquire_lock()
             time.sleep(10)
 
         return self
 
-    def release_lock(self) -> None:
+    def _release_lock(self) -> None:
         """ Release the lock by deleting the lock file
         """
         if self.has_lock() and self._fd:
             fcntl.flock(self._fd, fcntl.LOCK_UN)
             self._fd.close()
+            self._fd = None
             self._has_lock = False
             logger.debug(
                 "Removed lock from file %s.", str(self._lock_file_path)
             )
 
     def __enter__(self):
-        self.acquire_lock()
+        self._acquire_lock()
         return self
 
     def __exit__(self, exc_type, exc_value, exc_tb):
-        self.release_lock()
+        self._release_lock()

tests/test_lock.py

@@ -39,47 +39,47 @@ def test_acquire_lock(self):
         lock_file_path = self.temp_dir / 'test.lock'
 
         lock_file = LockFile(lock_file_path)
-        lock_file.acquire_lock()
+        lock_file._acquire_lock()
 
         self.assertTrue(lock_file.has_lock())
         self.assertTrue(lock_file_path.exists())
-        lock_file.release_lock()
+        lock_file._release_lock()
 
     @patch('ospd_openvas.lock.logger')
     def test_already_locked(self, mock_logger):
         lock_file_path = self.temp_dir / 'test.lock'
 
         lock_file_aux = LockFile(lock_file_path)
-        lock_file_aux.acquire_lock()
+        lock_file_aux._acquire_lock()
         self.assertTrue(lock_file_aux.has_lock())
 
         lock_file = LockFile(lock_file_path)
-        lock_file.acquire_lock()
+        lock_file._acquire_lock()
         self.assertFalse(lock_file.has_lock())
         assert_called_once(mock_logger.error)
 
-        lock_file_aux.release_lock()
+        lock_file_aux._release_lock()
 
     def test_create_parent_dirs(self):
         lock_file_path = self.temp_dir / 'foo' / 'bar' / 'test.lock'
 
         lock_file = LockFile(lock_file_path)
-        lock_file.acquire_lock()
+        lock_file._acquire_lock()
 
         self.assertTrue(lock_file.has_lock())
 
         self.assertTrue(lock_file_path.exists())
         self.assertTrue(lock_file_path.parent.is_dir())
         self.assertTrue(lock_file_path.parent.parent.is_dir())
 
-        lock_file.release_lock()
+        lock_file._release_lock()
 
     @patch('ospd_openvas.lock.logger')
     def test_create_paren_dirs_fail(self, mock_logger):
         lock_file_path = Path('/root/lock/file/test.lock')
         lock_file = LockFile(lock_file_path)
 
-        lock_file.acquire_lock()
+        lock_file._acquire_lock()
         self.assertFalse(lock_file.has_lock())
 
         assert_called_once(mock_logger.error)
@@ -92,7 +92,7 @@ def test_context_manager(self):
         with lock_file:
             self.assertTrue(lock_file.has_lock())
             self.assertTrue(lock_file_path.is_file())
-            lock_file.release_lock()
+            lock_file._release_lock()
 
         # The file is not removed
         self.assertFalse(lock_file.has_lock())