Fix #77837: Error removing addons/extension/wheels on WIN32

When deleting files on WIN32, open files cannot be removed.
This is especially a problem for compiled Python modules which
remain open once imported.

Previously it was not as common for add-ons to include compiled Python
modules however with extensions supporting Python-wheels,
it's increasingly likely users run into this.

Workaround the problem by:
- Scheduling the files for removal next time Blender starts.
- Rename paths that cannot be removed to avoid collisions when
  the paths is reused (re-installing for example).

This is supported for:
- Extensions.
- Python wheels.
- Legacy user add-ons.
- App-templates.

Details:
- On startup, a file exists that indicates cleanup is needed.
  In the common case the file doesn't exist.
  Otherwise module paths are scanned for files to remove.
- Since errors resolving paths to remove could result in user data loss,
  ensure the paths are always within the (extension/addon/app-template)
  directory.
- File locking isn't used, if multiple Blender instances start at the
  same time and try to remove the same files, this won't cause errors.
  Even so, remove the checking file immediately avoid unnecessary
  file-system access overhead for other Blender instances.

Also resolves #125049.

scripts/modules/_bpy_internal/extensions/stale_file_manager.py

@@ -0,0 +1,380 @@
+# SPDX-FileCopyrightText: 2024 Blender Foundation
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+# Schedule files for later removal, needed for situations where files are locked.
+#
+# This is mainly a workaround for WIN32 error where an add-on DLL
+# is considered *used* making it impossible to remove.
+#
+# This is also used on other systems as permissions can also prevent sub-directories from removed.
+# In this case renaming can make way the path to be replaced however it doesn't address
+# the problem of the "stale" path failing to be removed.
+# The user would need to change the permissions in this case (although this really a corner case).
+__all__ = (
+    "StaleFiles",
+)
+
+
+from typing import (
+    Dict,
+    List,
+    Sequence,
+)
+
+# The stale file-format is very simple and works as follows.
+#
+# - Every line references a path relative to the stale file.
+# - Paths must always references files within this directory
+#   (anything else must be ignored).
+# - Paths must always use forward slashes (even on WIN32).
+#   This is done since a repository may be accessed from different systems.
+# - Paths must end with a newline `\n`.
+#
+# Further notes:
+# - Corrupted "stale" files must be handled gracefully (it may be random bytes).
+# - Non UTF8 characters in paths are supported via `surrogateescape`.
+# - File names containing newlines are *not* supported.
+
+
+class StaleFiles:
+    __slots__ = (
+        # Files outside of this directory must *never* be removed.
+        "_base_directory",
+        # The name (within `_base_directory`) to load/store paths.
+        "_stale_filename",
+        # Stale paths relative to `_base_directory`.
+        "_paths",
+        # When true, print extra debug output.
+        "_debug",
+        # Store the cache index per-directory, avoids looking up an index every time a stale name needs to be created.
+        "_index_cache",
+        # True when the run-time state is different to the on-disk state.
+        "_is_modified",
+    )
+
+    def __init__(
+            self,
+            base_directory: str,
+            *,
+            stale_filename: str,
+            debug: bool = False,
+    ):
+        import os
+        from os import sep
+        assert base_directory not in ("", ".", "..")
+        # NOTE: on WIN32 `normpath` won't remove the trailing `sep`,
+        # it's important to add only if it's not there.
+        base_directory = os.path.normpath(base_directory)
+        self._base_directory = base_directory if base_directory.endswith(sep) else (base_directory + sep)
+        self._stale_filename = stale_filename
+        self._paths: List[str] = []
+        self._debug: bool = debug
+
+        self._index_cache: Dict[str, int] = {}
+        self._is_modified: bool = True
+
+    def is_empty(self) -> bool:
+        return not bool(self._paths)
+
+    def is_modified(self) -> bool:
+        return self._is_modified
+
+    def state_load(self, *, check_exists: bool) -> None:
+        import contextlib
+        import os
+        from os import sep
+
+        base_directory = self._base_directory
+        paths = self._paths
+        debug = self._debug
+
+        assert base_directory.endswith(sep)
+        # Don't support loading multiple times or after adding files.
+        assert len(paths) == 0
+
+        stale_filepath = os.path.join(base_directory, self._stale_filename)
+
+        line_count = 0
+
+        # Set here before early exit.
+        # Assume modified so any corrupt causes a re-write.
+        self._is_modified = True
+
+        try:
+            # pylint: disable-next=consider-using-with
+            fh_context = open(stale_filepath, "r", encoding="utf8", errors="surrogateescape")
+        except FileNotFoundError:
+            self._is_modified = False
+            return
+        except Exception as ex:
+            if debug:
+                print(base_directory, "error opening file for read", str(ex))
+            return
+
+        with contextlib.closing(fh_context) as fh:
+            fh_iter = iter(fh)
+            while True:
+                try:
+                    path = next(fh_iter)
+                except StopIteration:
+                    break
+                except Exception as ex:
+                    if debug:
+                        print(base_directory, "error reading line", str(ex))
+                    break
+
+                line_count += 1
+                # Not expected, file may be truncated.
+                if not path.endswith("\n"):
+                    if debug:
+                        print(base_directory, "expected line endings on each line")
+                    continue
+                path = path[:-1]
+                # Not expected but harmless, ignore if it does.
+                if not path:
+                    if debug:
+                        print(base_directory, "expected line not to be empty")
+                    continue
+
+                path_abs = base_directory + (path if sep == "/" else path.replace("/", "\\"))
+
+                if check_exists:
+                    # Harmless, somehow the file was removed.
+                    if not os.path.exists(path_abs):
+                        continue
+
+                path_abs = os.path.normpath(path_abs)
+                # Not expected, ensure under *no* conditions paths outside this directory are removed.
+                if not path_abs.startswith(base_directory):
+                    if debug:
+                        print(base_directory, "stale file points to parent path (unexpected but harmless)", repr(path))
+                    continue
+
+                # Ensure the `base_directory` & `path_abs` they are not the same.
+                # One could be forgiven for thinking they must never be the same since `path`
+                # is known not be be an empty string, one would be mistaken!
+                # WIN32 which considers `C:\path\` the same as `C:\path\. ` to be the same.
+                # Therefor, literal lines containing any combination of trailing full-stop
+                # or space characters would be considered files that cannot be removed.
+                # While this should never under normal conditions happen,
+                # guarantee that stale file removal *never* removes anything it should not,
+                # including situations when random bytes are written into this file
+                # (except in the case the random bytes happen to match a patch - which can't be avoided).
+                #
+                # If this ever did happen besides potentially trying to remove `base_directory`,
+                # this path could be treated as a file which could not be removed and queued for
+                # removal again causing a single space (for e.g.) to be left in the stale file,
+                # trying to be removed every startup and failing.
+                # Avoid all these issues by checking the path doesn't resolve to being the same path as it's parent.
+                is_same = False
+                try:
+                    is_same = os.path.samefile(base_directory, path_abs)
+                except FileNotFoundError:
+                    pass
+                except Exception as ex:
+                    if debug:
+                        print(base_directory, "error checking the same path", str(ex))
+
+                if is_same:
+                    if debug:
+                        print(base_directory, "path results to it's parent", repr(path))
+                    continue
+
+                # NOTE: duplicates are not checked, while they aren't expected, duplicates won't cause errors.
+                paths.append(path)
+
+        self._is_modified = len(paths) != line_count
+
+    def state_store(self, *, check_exists: bool) -> None:
+        import contextlib
+        import os
+        from os import sep
+
+        base_directory = self._base_directory
+        debug = self._debug
+
+        stale_filepath = os.path.join(base_directory, self._stale_filename)
+
+        if not self._paths:
+            self._is_modified = False
+            try:
+                os.remove(stale_filepath)
+            except FileNotFoundError:
+                pass
+            except Exception as ex:
+                if debug:
+                    print(base_directory, "failed to remove!", str(ex))
+                self._is_modified = True
+
+            return
+
+        try:
+            # pylint: disable-next=consider-using-with
+            fh_context = open(stale_filepath, "w", encoding="utf8", errors="surrogateescape")
+        except Exception as ex:
+            if debug:
+                print(base_directory, "error opening file for write", str(ex))
+            self._is_modified = True
+            return
+
+        # Assume success, any errors can set to true.
+        is_modified = False
+
+        with contextlib.closing(fh_context) as fh:
+            for path in self._paths:
+                if check_exists:
+                    path_abs = base_directory + (path if sep == "/" else path.replace("/", "\\"))
+                    # Harmless, somehow the file was removed.
+                    if not os.path.exists(path_abs):
+                        continue
+
+                try:
+                    fh.write(path + "\n")
+                except Exception as ex:
+                    if debug:
+                        print(base_directory, "failed to write path", str(ex))
+                    is_modified = True
+                    break
+
+        self._is_modified = is_modified
+
+    def state_remove_all(self) -> bool:
+        import stat
+        import shutil
+        import os
+
+        from os import sep
+
+        base_directory = self._base_directory
+        debug = self._debug
+
+        paths_next = []
+
+        for path in self._paths:
+            path_abs = base_directory + (path if sep == "/" else path.replace("/", "\\"))
+            path_abs = os.path.normpath(path_abs)
+
+            # Should be unreachable, extra paranoid check so we *never*
+            # recursively remove anything outside of the base directory.
+            if not path_abs.startswith(base_directory):
+                print("Internal error detected attempting to remove file outside of:", base_directory)
+                continue
+
+            try:
+                st = os.stat(path_abs)
+            except FileNotFoundError:
+                # Not a problem if it's already removed.
+                continue
+            except Exception as ex:
+                if debug:
+                    print(base_directory, "failed to stat file", path, str(ex))
+                continue
+
+            if stat.S_ISDIR(st.st_mode):
+                try:
+                    shutil.rmtree(path_abs)
+                except Exception as ex:
+                    # May be necessary with links.
+                    try:
+                        os.remove(path_abs)
+                    except Exception:
+                        if debug:
+                            print(base_directory, "failed to remove dir", path, str(ex))
+            else:
+                try:
+                    os.remove(path_abs)
+                except Exception as ex:
+                    if debug:
+                        print(base_directory, "failed to remove file", path, str(ex))
+
+            # Failed to remove, add back to the list.
+            if os.path.exists(path_abs):
+                paths_next.append(path)
+
+        if len(self._paths) == len(paths_next):
+            return False
+
+        self._is_modified = True
+        self._paths[:] = paths_next
+        return True
+
+    def state_load_add_and_store(
+            self,
+            *,
+            # A sequence of absolute paths within `_base_directory`.
+            paths: Sequence[str],
+    ) -> bool:
+        # Convenience function for a common operation.
+        # Return true when one or more items from "paths" were added to the "state".
+
+        self.state_load(check_exists=True)
+        if not self.is_empty():
+            self.state_remove_all()
+
+        result = False
+        for path_abs in paths:
+            self.filepath_add(path_abs, rename=True)
+            result = True
+
+        if self.is_modified():
+            self.state_store(check_exists=False)
+
+        return result
+
+    def _filepath_rename_to_stale(self, path_abs: str) -> str:
+        import os
+
+        base_directory = self._base_directory
+        debug = self._debug
+
+        # These need not necessarily match, it could be optional.
+        prefix = self._stale_filename
+
+        dirpath = os.path.dirname(path_abs)
+        stale_index = self._index_cache.get(dirpath, 1)
+        while True:
+            path_abs_stale = os.path.join(dirpath, "{:s}{:04x}".format(prefix, stale_index))
+            if not os.path.exists(path_abs_stale):
+                break
+            stale_index += 1
+
+        rename_ok = False
+        try:
+            os.rename(path_abs, path_abs_stale)
+            rename_ok = True
+        except Exception as ex:
+            if debug:
+                print(base_directory, "failed to rename path", str(ex))
+
+        if rename_ok:
+            self._index_cache[dirpath] = stale_index + 1
+        else:
+            # Failed to rename, make the previous name stale as we have no better options.
+            path_abs_stale = path_abs
+            if debug:
+                print("failed to rename:", path_abs)
+
+        return path_abs_stale
+
+    def filepath_add(self, path_abs: str, *, rename: bool) -> bool:
+        from os import sep
+
+        base_directory = self._base_directory
+        debug = self._debug
+
+        if not path_abs.startswith(self._base_directory):
+            if debug:
+                print(base_directory, "is not a sub-directory", path_abs)
+            return False
+
+        if rename:
+            path_abs = self._filepath_rename_to_stale(path_abs)
+
+        path = path_abs[len(self._base_directory):].lstrip(sep)
+        if sep == "\\":
+            path = path.replace("\\", "/")
+
+        self._is_modified = True
+        self._paths.append(path)
+        return True

scripts/modules/_bpy_internal/extensions/wheel_manager.py

@@ -16,9 +16,11 @@ __all__ = (
 import os
 import re
 import shutil
+import sys
 import zipfile
 
 from typing import (
+    Callable,
     Dict,
     List,
     Optional,
@@ -139,10 +141,39 @@ def _wheel_info_dir_from_zip(filepath_wheel: str) -> Optional[Tuple[str, List[st
     return dir_info, toplevel_paths_list
 
 
-def _rmtree_safe(dir_remove: str, expected_root: str) -> None:
+def _rmtree_safe(dir_remove: str, expected_root: str) -> Optional[Exception]:
     if not dir_remove.startswith(expected_root):
         raise Exception("Expected prefix not found")
-    shutil.rmtree(dir_remove)
+
+    ex_result = None
+
+    if sys.version_info < (3, 12):
+        def on_error(*args) -> None:  # type: ignore
+            nonlocal ex_result
+            print("Failed to remove:", args)
+            ex_result = args[2][0]
+
+        shutil.rmtree(dir_remove, onerror=on_error)
+    else:
+        def on_exc(*args) -> None:  # type: ignore
+            nonlocal ex_result
+            print("Failed to remove:", args)
+            ex_result = args[2]
+
+        shutil.rmtree(dir_remove, onexc=on_exc)
+
+    return ex_result
+
+
+def _remove_safe(file_remove: str) -> Optional[Exception]:
+    ex_result = None
+
+    try:
+        os.remove(file_remove)
+    except Exception as ex:
+        ex_result = ex
+
+    return ex_result
 
 
 def _zipfile_extractall_safe(
@@ -281,6 +312,8 @@ def apply_action(
         local_dir: str,
         local_dir_site_packages: str,
         wheel_list: List[WheelSource],
+        remove_error_fn: Callable[[str, Exception], None],
+        debug: bool,
 ) -> None:
     """
     :arg local_dir:
@@ -292,7 +325,6 @@ def apply_action(
        The path which wheels are extracted into.
        Typically: ``~/.config/blender/4.2/extensions/.local/lib/python3.11/site-packages``.
     """
-    debug = False
 
     # NOTE: we could avoid scanning the wheel directories however:
     # Recursively removing all paths on the users system can be considered relatively risky
@@ -357,10 +389,22 @@ def apply_action(
             if debug:
                 print("removing wheel:", filepath_rel)
 
+            ex: Optional[Exception] = None
             if os.path.isdir(filepath_abs):
-                _rmtree_safe(filepath_abs, local_dir)
+                ex = _rmtree_safe(filepath_abs, local_dir)
+                # For symbolic-links, use remove as a fallback.
+                if ex is not None:
+                    if _remove_safe(filepath_abs) is None:
+                        ex = None
             else:
-                os.remove(filepath_abs)
+                ex = _remove_safe(filepath_abs)
+
+            if ex:
+                if debug:
+                    print("failed to remove:", filepath_rel, str(ex), "setting stale")
+
+                # If the directory (or file) can't be removed, make it stale and try to remove it later.
+                remove_error_fn(filepath_abs, ex)
 
     # -----
     # Setup

scripts/modules/addon_utils.py

@@ -13,6 +13,7 @@ __all__ = (
     "reset_all",
     "module_bl_info",
     "extensions_refresh",
+    "stale_pending_stage_paths",
 )
 
 import bpy as _bpy
@@ -30,12 +31,17 @@ _extensions_incompatible = {}
 # `{addon_module_name: "Warning", ...}`
 _extensions_warnings = {}
 
+# Filename used for stale files (which we can't delete).
+_stale_filename = ".~stale~"
+
 
 # called only once at startup, avoids calling 'reset_all', correct but slower.
 def _initialize_once():
     for path in paths():
         _bpy.utils._sys_path_ensure_append(path)
 
+    _stale_pending_check_and_remove_once()
+
     _initialize_extensions_repos_once()
 
     for addon in _preferences.addons:
@@ -678,6 +684,137 @@ def module_bl_info(mod, *, info_basis=None):
     return addon_info
 
 
+# -----------------------------------------------------------------------------
+# Stale File Handling
+#
+# Notes:
+# - On startup, a file exists that indicates cleanup is needed.
+#   In the common case the file doesn't exist.
+#   Otherwise module paths are scanned for files to remove.
+# - Since errors resolving paths to remove could result in user data loss,
+#   ensure the paths are always within the (extension/add-on/app-template) directory.
+# - File locking isn't used, if multiple Blender instances start at the
+#   same time and try to remove the same files, this won't cause errors.
+#   Even so, remove the checking file immediately avoid unnecessary
+#   file-system access overhead for other Blender instances.
+#
+# For more implementation details see `_bpy_internal.extensions.stale_file_manager`.
+# This mainly impacts WIN32 which can't remove open file handles, see: #77837 & #125049.
+#
+# Use for all systems as the problem can impact any system if file removal fails
+# for any reason (typically permissions or file-system error).
+
+def _stale_pending_filepath():
+    # When this file exists, stale file removal is pending.
+    # Try to remove stale files next launch.
+    import os
+    return os.path.join(_bpy.utils.user_resource('CONFIG'), "stale-pending")
+
+
+def _stale_pending_stage(debug):
+    import os
+
+    stale_pending_filepath = _stale_pending_filepath()
+    dirpath = os.path.dirname(stale_pending_filepath)
+
+    if os.path.exists(stale_pending_filepath):
+        return
+
+    try:
+        os.makedirs(dirpath, exist_ok=True)
+        with open(stale_pending_filepath, "wb") as _:
+            pass
+    except Exception as ex:
+        print("Unable to set stale files pending:", str(ex))
+
+
+def _stale_file_directory_iter():
+    import os
+
+    for repo in _preferences.extensions.repos:
+        if not repo.enabled:
+            continue
+        if repo.source == 'SYSTEM':
+            continue
+        yield repo.directory
+
+    # Skip `addons_core` because add-ons because these will never be uninstalled by the user.
+    yield from paths()[1:]
+
+    # The `local_dir`, for wheels.
+    yield os.path.join(_bpy.utils.user_resource('EXTENSIONS'), ".local")
+
+    # The `path_app_templates`, for user app-templates.
+    yield _bpy.utils.user_resource(
+        'SCRIPTS',
+        path=os.path.join("startup", "bl_app_templates_user"),
+        create=False,
+    )
+
+
+def _stale_pending_check_and_remove_once():
+    # This runs on every startup, early exit if no stale data removal is staged.
+    import os
+    stale_pending_filepath = _stale_pending_filepath()
+    if not os.path.exists(stale_pending_filepath):
+        return
+
+    # Some stale data needs to be removed, this is an exceptional case.
+    # Allow for slower logic that is typically accepted on startup.
+    from _bpy_internal.extensions.stale_file_manager import StaleFiles
+    debug = _bpy.app.debug_python
+
+    # Remove the pending file if all are removed.
+    is_empty = True
+
+    for dirpath in _stale_file_directory_iter():
+        if not os.path.exists(os.path.join(dirpath, _stale_filename)):
+            continue
+
+        try:
+            stale_handle = StaleFiles(
+                base_directory=dirpath,
+                stale_filename=_stale_filename,
+                debug=debug,
+            )
+            stale_handle.state_load(check_exists=True)
+            if not stale_handle.is_empty():
+                stale_handle.state_remove_all()
+                if not stale_handle.is_empty():
+                    is_empty = False
+            if stale_handle.is_modified():
+                stale_handle.state_store(check_exists=False)
+        except Exception as ex:
+            print("Unexpected error clearing stale data, this is is a bug!", str(ex))
+
+    if is_empty:
+        try:
+            os.remove(stale_pending_filepath)
+        except Exception as ex:
+            if debug:
+                print("Failed to remove stale-pending file:", str(ex))
+
+
+def stale_pending_stage_paths(path_base, paths):
+    # - `path_base` must a directory iterated over by `_stale_file_directory_iter`.
+    #   Otherwise the stale files will never be removed.
+    # - `paths` must be absolute paths which could not be removed.
+    #   They must be located within `base_path` otherwise they cannot be removed.
+    from _bpy_internal.extensions.stale_file_manager import StaleFiles
+
+    debug = _bpy.app.debug_python
+
+    stale_handle = StaleFiles(
+        base_directory=path_base,
+        stale_filename=_stale_filename,
+        debug=debug,
+    )
+    # Already checked.
+    if stale_handle.state_load_add_and_store(paths=paths):
+        # Force clearing stale files on next restart.
+        _stale_pending_stage(debug)
+
+
 # -----------------------------------------------------------------------------
 # Extension Pre-Flight Compatibility Check
 #
@@ -958,11 +1095,12 @@ def _initialize_extensions_compat_ensure_up_to_date(extensions_directory, extens
     return updated, wheel_list
 
 
-def _initialize_extensions_compat_ensure_up_to_date_wheels(extensions_directory, wheel_list):
+def _initialize_extensions_compat_ensure_up_to_date_wheels(extensions_directory, wheel_list, debug):
     import os
     _extension_sync_wheels(
         local_dir=os.path.join(extensions_directory, ".local"),
         wheel_list=wheel_list,
+        debug=debug,
     )
 
 
@@ -987,10 +1125,8 @@ def _initialize_extensions_compat_data(extensions_directory, ensure_wheels, addo
             if check_extension(module_name):
                 extensions_enabled.add(module_name[extensions_prefix_len:].partition(".")[0::2])
 
-    print_debug = (
-        (lambda *args, **kwargs: print("Extension version cache:", *args, **kwargs)) if _bpy.app.debug_python else
-        None
-    )
+    debug = _bpy.app.debug_python
+    print_debug = (lambda *args, **kwargs: print("Extension version cache:", *args, **kwargs)) if debug else None
 
     # Early exit, use for automated tests.
     # Avoid (relatively) expensive file-system scanning if at all possible.
@@ -1016,7 +1152,7 @@ def _initialize_extensions_compat_data(extensions_directory, ensure_wheels, addo
     if ensure_wheels:
         if updated:
             try:
-                _initialize_extensions_compat_ensure_up_to_date_wheels(extensions_directory, wheel_list)
+                _initialize_extensions_compat_ensure_up_to_date_wheels(extensions_directory, wheel_list, debug)
             except Exception as ex:
                 print("Extension: unexpected error updating wheels, this is is a bug!")
                 import traceback
@@ -1138,6 +1274,7 @@ def _extension_sync_wheels(
         *,
         local_dir,  # `str`
         wheel_list,  # `List[WheelSource]`
+        debug,           # `bool`
 ):  # `-> None`
     import os
     import sys
@@ -1150,11 +1287,22 @@ def _extension_sync_wheels(
         "site-packages",
     )
 
+    paths_stale = []
+
+    def remove_error_fn(filepath: str, _ex: Exception) -> None:
+        paths_stale.append(filepath)
+
     apply_action(
         local_dir=local_dir,
         local_dir_site_packages=local_dir_site_packages,
         wheel_list=wheel_list,
+        remove_error_fn=remove_error_fn,
+        debug=debug,
     )
+
+    if paths_stale:
+        stale_pending_stage_paths(local_dir, paths_stale)
+
     if os.path.exists(local_dir_site_packages):
         if local_dir_site_packages not in sys.path:
             sys.path.append(local_dir_site_packages)