add: File IO timeout API (#133)

Born from the discussion in #129 and continued in #130. This adds timeout-oriented functions for reading and writing to the scanned source files.

This supersedes #130 and resolves #129.

* run clang-format before clang-tidy
* prevent busy waits and log errors

Author: 2bndy5

cpp_linter/clang_tools/__init__.py

@@ -6,7 +6,7 @@
 from typing import Optional, List, Dict, Tuple, cast
 import shutil
 
-from ..common_fs import FileObj
+from ..common_fs import FileObj, FileIOTimeout
 from ..common_fs.file_filter import TidyFileFilter, FormatFileFilter
 from ..loggers import start_log_group, end_log_group, worker_log_init, logger
 from .clang_tidy import run_clang_tidy, TidyAdvice
@@ -45,34 +45,52 @@ def _run_on_single_file(
     args: Args,
 ) -> Tuple[str, str, Optional[TidyAdvice], Optional[FormatAdvice]]:
     log_stream = worker_log_init(log_lvl)
-
-    tidy_note = None
-    if tidy_cmd is not None and (
-        tidy_filter is None or tidy_filter.is_source_or_ignored(file.name)
-    ):
-        tidy_note = run_clang_tidy(
-            command=tidy_cmd,
-            file_obj=file,
-            checks=args.tidy_checks,
-            lines_changed_only=args.lines_changed_only,
-            database=args.database,
-            extra_args=args.extra_arg,
-            db_json=db_json,
-            tidy_review=args.tidy_review,
-            style=args.style,
-        )
+    filename = Path(file.name).as_posix()
 
     format_advice = None
     if format_cmd is not None and (
         format_filter is None or format_filter.is_source_or_ignored(file.name)
     ):
-        format_advice = run_clang_format(
-            command=format_cmd,
-            file_obj=file,
-            style=args.style,
-            lines_changed_only=args.lines_changed_only,
-            format_review=args.format_review,
-        )
+        try:
+            format_advice = run_clang_format(
+                command=format_cmd,
+                file_obj=file,
+                style=args.style,
+                lines_changed_only=args.lines_changed_only,
+                format_review=args.format_review,
+            )
+        except FileIOTimeout:  # pragma: no cover
+            logger.error(
+                "Failed to read or write contents of %s when running clang-format",
+                filename,
+            )
+        except OSError:  # pragma: no cover
+            logger.error(
+                "Failed to open the file %s when running clang-format", filename
+            )
+
+    tidy_note = None
+    if tidy_cmd is not None and (
+        tidy_filter is None or tidy_filter.is_source_or_ignored(file.name)
+    ):
+        try:
+            tidy_note = run_clang_tidy(
+                command=tidy_cmd,
+                file_obj=file,
+                checks=args.tidy_checks,
+                lines_changed_only=args.lines_changed_only,
+                database=args.database,
+                extra_args=args.extra_arg,
+                db_json=db_json,
+                tidy_review=args.tidy_review,
+                style=args.style,
+            )
+        except FileIOTimeout:  # pragma: no cover
+            logger.error(
+                "Failed to Read/Write contents of %s when running clang-tidy", filename
+            )
+        except OSError:  # pragma: no cover
+            logger.error("Failed to open the file %s when running clang-tidy", filename)
 
     return file.name, log_stream.getvalue(), tidy_note, format_advice
 

cpp_linter/clang_tools/clang_format.py

@@ -130,12 +130,13 @@ def parse_format_replacements_xml(
         file_obj.range_of_changed_lines(lines_changed_only, get_ranges=True),
     )
     tree = ET.fromstring(xml_out)
+    content = file_obj.read_with_timeout()
     for child in tree:
         if child.tag == "replacement":
             null_len = int(child.attrib["length"])
             text = "" if child.text is None else child.text
             offset = int(child.attrib["offset"])
-            line, cols = get_line_cnt_from_cols(file_obj.name, offset)
+            line, cols = get_line_cnt_from_cols(content, offset)
             is_line_in_ranges = False
             for r in ranges:
                 if line in range(r[0], r[1]):  # range is inclusive

cpp_linter/clang_tools/clang_tidy.py

@@ -246,7 +246,7 @@ def run_clang_tidy(
     if tidy_review:
         # clang-tidy overwrites the file contents when applying fixes.
         # create a cache of original contents
-        original_buf = Path(file_obj.name).read_bytes()
+        original_buf = file_obj.read_with_timeout()
         cmds.append("--fix-errors")  # include compiler-suggested fixes
     cmds.append(filename)
     logger.info('Running "%s"', " ".join(cmds))
@@ -260,10 +260,8 @@ def run_clang_tidy(
     advice = parse_tidy_output(results.stdout.decode(), database=db_json)
 
     if tidy_review:
-        # store the modified output from clang-tidy
-        advice.patched = Path(file_obj.name).read_bytes()
-        # re-write original file contents
-        Path(file_obj.name).write_bytes(original_buf)
+        # store the modified output from clang-tidy and re-write original file contents
+        advice.patched = file_obj.read_write_with_timeout(original_buf)
 
     return advice
 

cpp_linter/clang_tools/patcher.py

@@ -2,7 +2,6 @@
 by the clang tool's output."""
 
 from abc import ABC
-from pathlib import Path
 from typing import Optional, Dict, Any, List, Tuple
 from pygit2 import Patch  # type: ignore
 from ..common_fs import FileObj
@@ -178,7 +177,7 @@ def get_suggestions_from_patch(
             self.patched
         ), f"{self.__class__.__name__} has no suggestions for {file_obj.name}"
         patch = Patch.create_from(
-            Path(file_obj.name).read_bytes(),
+            file_obj.read_with_timeout(),
             self.patched,
             file_obj.name,
             file_obj.name,

cpp_linter/common_fs/__init__.py

@@ -1,5 +1,6 @@
 from os import environ
 from pathlib import Path
+import time
 from typing import List, Dict, Any, Union, Tuple, Optional, TYPE_CHECKING
 from pygit2 import DiffHunk  # type: ignore
 from ..loggers import logger
@@ -158,6 +159,90 @@ def is_range_contained(self, start: int, end: int) -> Optional[Tuple[int, int]]:
         )
         return None
 
+    def read_with_timeout(self, timeout_ns: int = 1_000_000_000) -> bytes:
+        """Read the entire file's contents.
+
+        :param timeout_ns: The number of nanoseconds to wait till timeout occurs.
+            Defaults to 1 second.
+
+        :returns: The bytes read from the file.
+
+        :raises FileIOTimeout: When the operation did not succeed due to a timeout.
+        :raises OSError: When the file could not be opened due to an `OSError`.
+        """
+        contents = b""
+        success = False
+        exception: Union[OSError, FileIOTimeout] = FileIOTimeout(
+            f"Failed to read from file '{self.name}' within "
+            + f"{round(timeout_ns / 1_000_000_000, 2)} seconds"
+        )
+        timeout = time.monotonic_ns() + timeout_ns
+        while not success and time.monotonic_ns() < timeout:
+            try:
+                with open(self.name, "rb") as f:
+                    while not success and time.monotonic_ns() < timeout:
+                        if f.readable():
+                            contents = f.read()
+                            success = True
+                        else:  # pragma: no cover
+                            time.sleep(0.001)  # Sleep to prevent busy-waiting
+            except OSError as exc:  # pragma: no cover
+                exception = exc
+        if not success and exception:  # pragma: no cover
+            raise exception
+        return contents
+
+    def read_write_with_timeout(
+        self,
+        data: Union[bytes, bytearray],
+        timeout_ns: int = 1_000_000_000,
+    ) -> bytes:
+        """Read then write the entire file's contents.
+
+        :param data: The bytes to write to the file. This will overwrite the contents
+            being read beforehand.
+        :param timeout_ns: The number of nanoseconds to wait till timeout occurs.
+            Defaults to 1 second.
+
+        :returns: The bytes read from the file.
+
+        :raises FileIOTimeout: When the operation did not succeed due to a timeout.
+        :raises OSError: When the file could not be opened due to an `OSError`.
+        """
+        success = False
+        exception: Union[OSError, FileIOTimeout] = FileIOTimeout(
+            f"Failed to read then write file '{self.name}' within "
+            + f"{round(timeout_ns / 1_000_000_000, 2)} seconds"
+        )
+        original_data = b""
+        timeout = time.monotonic_ns() + timeout_ns
+        while not success and time.monotonic_ns() < timeout:
+            try:
+                with open(self.name, "r+b") as f:
+                    while not success and time.monotonic_ns() < timeout:
+                        if f.readable():
+                            original_data = f.read()
+                            f.seek(0)
+                        else:  # pragma: no cover
+                            time.sleep(0.001)  # Sleep to prevent busy-waiting
+                            continue
+                        while not success and time.monotonic_ns() < timeout:
+                            if f.writable():
+                                f.write(data)
+                                f.truncate()
+                                success = True
+                            else:  # pragma: no cover
+                                time.sleep(0.001)  # Sleep to prevent busy-waiting
+            except OSError as exc:  # pragma: no cover
+                exception = exc
+        if not success and exception:  # pragma: no cover
+            raise exception
+        return original_data
+
+
+class FileIOTimeout(Exception):
+    """An exception thrown when a file operation timed out."""
+
 
 def has_line_changes(
     lines_changed_only: int, diff_chunks: List[List[int]], additions: List[int]
@@ -180,10 +265,10 @@ def has_line_changes(
     )
 
 
-def get_line_cnt_from_cols(file_path: str, offset: int) -> Tuple[int, int]:
+def get_line_cnt_from_cols(data: bytes, offset: int) -> Tuple[int, int]:
     """Gets a line count and columns offset from a file's absolute offset.
 
-    :param file_path: Path to file.
+    :param data: Bytes content to analyze.
     :param offset: The byte offset to translate
 
     :returns:
@@ -193,5 +278,5 @@ def get_line_cnt_from_cols(file_path: str, offset: int) -> Tuple[int, int]:
         - Index 1 is the column number for the given offset on the line.
     """
     # logger.debug("Getting line count from %s at offset %d", file_path, offset)
-    contents = Path(file_path).read_bytes()[:offset]
+    contents = data[:offset]
     return (contents.count(b"\n") + 1, offset - contents.rfind(b"\n"))

tests/test_misc.py

@@ -84,8 +84,8 @@ def test_list_src_files(
 @pytest.mark.parametrize("line,cols,offset", [(13, 5, 144), (19, 1, 189)])
 def test_file_offset_translation(line: int, cols: int, offset: int):
     """Validate output from ``get_line_cnt_from_cols()``"""
-    test_file = str(Path("tests/demo/demo.cpp").resolve())
-    assert (line, cols) == get_line_cnt_from_cols(test_file, offset)
+    contents = Path("tests/demo/demo.cpp").read_bytes()
+    assert (line, cols) == get_line_cnt_from_cols(contents, offset)
 
 
 def test_serialize_file_obj():