Inject sys.frozen and sys.frozendllhandle as constructor arguments instead of referencing them directly in RegistryEntries. (#736)

* Make calling `_get_serverdll` in `RegistryEntries.__init__`.

* Make `serverdll` to optional kwargs of `RegistryEntries.__init__`.

* Make calling `_get_serverdll` in `Registrar.__init__`.

* Quit calling `_get_serverdll` in `RegistryEntries.__init__`.

* Make calling `getattr(sys, "frozen", None)` in `RegistryEntries.__init__`.

* Make calling `getattr(sys, "frozen", None)` in `Registrar.__init__` and make `frozen` to optional kwargs of `RegistryEntries.__init__`.

* Make calling `getattr(sys, "frozendllhandle", None)` in `RegistryEntries.__init__`.

* Make calling `getattr(sys, "frozendllhandle", None)` in `Registrar.__init__` and make `frozendllhandle` to optional kwargs of `RegistryEntries.__init__`.

* Replace calling `hasattr`s with referencing attributes in `Registrar`.

* Improve `_get_serverdll` and Remove assigning the `serverdll` instance variable.

* Fix a type annotation.

Author: junkmd

comtypes/server/register.py

@@ -99,6 +99,13 @@ class Registrar(object):
     work.
     """
 
+    _frozen: Optional[str]
+    _frozendllhandle: Optional[int]
+
+    def __init__(self) -> None:
+        self._frozen = getattr(sys, "frozen", None)
+        self._frozendllhandle = getattr(sys, "frozendllhandle", None)
+
     def nodebug(self, cls: Type) -> None:
         """Delete logging entries from the registry."""
         clsid = cls._reg_clsid_
@@ -154,7 +161,13 @@ def register(self, cls: Type, executable: Optional[str] = None) -> None:
             self._register(cls, executable)
 
     def _register(self, cls: Type, executable: Optional[str] = None) -> None:
-        table = sorted(RegistryEntries(cls))
+        table = sorted(
+            RegistryEntries(
+                cls,
+                frozen=self._frozen,
+                frozendllhandle=self._frozendllhandle,
+            )
+        )
         _debug("Registering %s", cls)
         for hkey, subkey, valuename, value in table:
             _debug("[%s\\%s]", _explain(hkey), subkey)
@@ -164,14 +177,14 @@ def _register(self, cls: Type, executable: Optional[str] = None) -> None:
 
         tlib = getattr(cls, "_reg_typelib_", None)
         if tlib is not None:
-            if hasattr(sys, "frozendllhandle"):
-                dll = _get_serverdll()
-                _debug("LoadTypeLibEx(%s, REGKIND_REGISTER)", dll)
-                LoadTypeLibEx(dll, REGKIND_REGISTER)
+            if self._frozendllhandle is not None:
+                frozen_dll = _get_serverdll(self._frozendllhandle)
+                _debug("LoadTypeLibEx(%s, REGKIND_REGISTER)", frozen_dll)
+                LoadTypeLibEx(frozen_dll, REGKIND_REGISTER)
             else:
                 if executable:
                     path = executable
-                elif hasattr(sys, "frozen"):
+                elif self._frozen is not None:
                     path = sys.executable
                 else:
                     path = cls._typelib_path_
@@ -190,7 +203,12 @@ def unregister(self, cls: Type, force: bool = False) -> None:
     def _unregister(self, cls: Type, force: bool = False) -> None:
         # If force==False, we only remove those entries that we
         # actually would have written.  It seems ATL does the same.
-        table = [t[:2] for t in RegistryEntries(cls)]
+        table = [
+            t[:2]
+            for t in RegistryEntries(
+                cls, frozen=self._frozen, frozendllhandle=self._frozendllhandle
+            )
+        ]
         # only unique entries
         table = list(set(table))
         table.sort()
@@ -221,17 +239,24 @@ def _unregister(self, cls: Type, force: bool = False) -> None:
         _debug("Done")
 
 
-def _get_serverdll() -> str:
+def _get_serverdll(handle: Optional[int]) -> str:
     """Return the pathname of the dll hosting the COM object."""
-    handle = getattr(sys, "frozendllhandle", None)
     if handle is not None:
         return GetModuleFileName(handle, 260)
     return _ctypes.__file__
 
 
 class RegistryEntries(object):
-    def __init__(self, cls: Type) -> None:
+    def __init__(
+        self,
+        cls: Type,
+        *,
+        frozen: Optional[str] = None,
+        frozendllhandle: Optional[int] = None,
+    ) -> None:
         self._cls = cls
+        self._frozen = frozen
+        self._frozendllhandle = frozendllhandle
 
     def _get_full_classname(self, cls: Type) -> str:
         """Return <modulename>.<classname> for 'cls'."""
@@ -312,11 +337,11 @@ def __iter__(self) -> Iterator[Tuple[int, str, str, str]]:
         localsvr_ctx = bool(clsctx & comtypes.CLSCTX_LOCAL_SERVER)
         inprocsvr_ctx = bool(clsctx & comtypes.CLSCTX_INPROC_SERVER)
 
-        if localsvr_ctx and not hasattr(sys, "frozendllhandle"):
+        if localsvr_ctx and self._frozendllhandle is None:
             exe = sys.executable
             if " " in exe:
                 exe = f'"{exe}"'
-            if not hasattr(sys, "frozen"):
+            if self._frozen is None:
                 if not __debug__:
                     exe = f"{exe} -O"
                 script = os.path.abspath(sys.modules[cls.__module__].__file__)  # type: ignore
@@ -328,11 +353,16 @@ def __iter__(self) -> Iterator[Tuple[int, str, str, str]]:
 
         # Register InprocServer32 only when run from script or from
         # py2exe dll server, not from py2exe exe server.
-        if inprocsvr_ctx and getattr(sys, "frozen", None) in (None, "dll"):
-            yield (HKCR, rf"CLSID\{reg_clsid}\InprocServer32", "", _get_serverdll())
+        if inprocsvr_ctx and self._frozen in (None, "dll"):
+            yield (
+                HKCR,
+                rf"CLSID\{reg_clsid}\InprocServer32",
+                "",
+                _get_serverdll(self._frozendllhandle),
+            )
             # only for non-frozen inproc servers the PythonPath/PythonClass is needed.
             if (
-                not hasattr(sys, "frozendllhandle")
+                self._frozendllhandle is None
                 or not comtypes.server.inprocserver._clsid_to_class
             ):
                 yield (

comtypes/test/test_server_register.py

@@ -191,15 +191,13 @@ def test_calls_cls_unregister(self):
 
 class Test_get_serverdll(ut.TestCase):
     def test_nonfrozen(self):
-        self.assertEqual(_ctypes.__file__, _get_serverdll())
+        self.assertEqual(_ctypes.__file__, _get_serverdll(None))
 
     @mock.patch.object(register, "GetModuleFileName")
-    @mock.patch.object(register, "sys")
-    def test_frozen(self, _sys, GetModuleFileName):
-        handle, dll_path = 1234, r"path\to\frozendll"
-        _sys.frozendllhandle = handle
+    def test_frozen(self, GetModuleFileName):
+        handle, dll_path = 1234, r"path\to\frozen.dll"
         GetModuleFileName.return_value = dll_path
-        self.assertEqual(r"path\to\frozendll", _get_serverdll())
+        self.assertEqual(dll_path, _get_serverdll(handle))
         (((hmodule, maxsize), _),) = GetModuleFileName.call_args_list
         self.assertEqual(handle, hmodule)
         self.assertEqual(260, maxsize)
@@ -409,60 +407,42 @@ class Cls:
 
 
 class Test_Frozen_RegistryEntries(ut.TestCase):
-    @mock.patch.object(register, "sys")
-    def test_local_dll(self, _sys):
-        _sys.mock_add_spec(["executable", "frozen"])
-        _sys.executable = sys.executable
-        _sys.frozen = "dll"
-        reg_clsid = GUID.create_new()
-        reg_clsctx = comtypes.CLSCTX_LOCAL_SERVER
+    SERVERDLL = r"my\target\server.dll"
 
-        class Cls:
-            _reg_clsid_ = reg_clsid
-            _reg_clsctx_ = reg_clsctx
+    # We do not test the scenario where `frozen` is `'dll'` but
+    # `frozendllhandle` is `None`, as it is not a situation
+    # we anticipate.
 
-        clsid_sub = rf"CLSID\{reg_clsid}"
-        expected = [
-            (HKCR, clsid_sub, "", ""),
-            (HKCR, rf"{clsid_sub}\LocalServer32", "", sys.executable),
-        ]
-        self.assertEqual(expected, list(RegistryEntries(Cls)))
-
-    @mock.patch.object(register, "sys")
-    def test_local_frozendllhandle(self, _sys):
-        _sys.mock_add_spec(["frozen", "frozendllhandle"])
-        _sys.frozen = "dll"
-        _sys.frozendllhandle = 1234
+    def test_local_dll(self):
         reg_clsid = GUID.create_new()
         reg_clsctx = comtypes.CLSCTX_LOCAL_SERVER
 
         class Cls:
             _reg_clsid_ = reg_clsid
             _reg_clsctx_ = reg_clsctx
 
+        # In such cases, the server does not start because the
+        # InprocServer32/LocalServer32 keys are not registered.
         expected = [(HKCR, rf"CLSID\{reg_clsid}", "", "")]
-        self.assertEqual(expected, list(RegistryEntries(Cls)))
+        entries = RegistryEntries(Cls, frozen="dll", frozendllhandle=1234)
+        self.assertEqual(expected, list(entries))
 
-    @mock.patch.object(register, "sys")
-    def test_inproc_windows_exe(self, _sys):
-        _sys.mock_add_spec(["frozen"])
-        _sys.frozen = "windows_exe"
+    def test_local_windows_exe(self):
         reg_clsid = GUID.create_new()
-        reg_clsctx = comtypes.CLSCTX_INPROC_SERVER
+        reg_clsctx = comtypes.CLSCTX_LOCAL_SERVER
 
         class Cls:
             _reg_clsid_ = reg_clsid
             _reg_clsctx_ = reg_clsctx
 
-        expected = [(HKCR, rf"CLSID\{reg_clsid}", "", "")]
-        self.assertEqual(expected, list(RegistryEntries(Cls)))
+        expected = [
+            (HKCR, rf"CLSID\{reg_clsid}", "", ""),
+            (HKCR, rf"CLSID\{reg_clsid}\LocalServer32", "", sys.executable),
+        ]
+        self.assertEqual(expected, list(RegistryEntries(Cls, frozen="windows_exe")))
 
-    @mock.patch.object(register, "_get_serverdll", lambda: r"my\target\server.dll")
-    @mock.patch.object(register, "sys")
-    def test_inproc_dll_frozendllhandle_clsid_to_class(self, _sys):
-        _sys.mock_add_spec(["frozen", "frozendllhandle"])
-        _sys.frozen = "dll"
-        _sys.frozendllhandle = 1234
+    @mock.patch.object(register, "_get_serverdll", return_value=SERVERDLL)
+    def test_inproc_dll_nonempty_clsid_to_class(self, get_serverdll):
         reg_clsid = GUID.create_new()
         reg_clsctx = comtypes.CLSCTX_INPROC_SERVER
 
@@ -474,43 +454,17 @@ class Cls:
         inproc_srv_sub = rf"{clsid_sub}\InprocServer32"
         expected = [
             (HKCR, clsid_sub, "", ""),
-            (HKCR, inproc_srv_sub, "", r"my\target\server.dll"),
+            (HKCR, inproc_srv_sub, "", self.SERVERDLL),
         ]
 
         with mock.patch.dict(comtypes.server.inprocserver._clsid_to_class):
             comtypes.server.inprocserver._clsid_to_class.update({5678: Cls})
-            self.assertEqual(expected, list(RegistryEntries(Cls)))
-
-    @mock.patch.object(register, "_get_serverdll", lambda: r"my\target\server.dll")
-    @mock.patch.object(register, "sys")
-    def test_inproc_dll(self, _sys):
-        _sys.mock_add_spec(["frozen", "modules"])
-        _sys.frozen = "dll"
-        _sys.modules = sys.modules
-        reg_clsid = GUID.create_new()
-        reg_clsctx = comtypes.CLSCTX_INPROC_SERVER
-
-        class Cls:
-            _reg_clsid_ = reg_clsid
-            _reg_clsctx_ = reg_clsctx
+            entries = RegistryEntries(Cls, frozen="dll", frozendllhandle=1234)
+            self.assertEqual(expected, list(entries))
+        get_serverdll.assert_called_once_with(1234)
 
-        clsid_sub = rf"CLSID\{reg_clsid}"
-        inproc_srv_sub = rf"{clsid_sub}\InprocServer32"
-        full_classname = f"{__name__}.Cls"
-        expected = [
-            (HKCR, clsid_sub, "", ""),
-            (HKCR, inproc_srv_sub, "", r"my\target\server.dll"),
-            (HKCR, inproc_srv_sub, "PythonClass", full_classname),
-            (HKCR, inproc_srv_sub, "PythonPath", os.path.dirname(__file__)),
-        ]
-        self.assertEqual(expected, list(RegistryEntries(Cls)))
-
-    @mock.patch.object(register, "_get_serverdll", lambda: r"my\target\server.dll")
-    @mock.patch.object(register, "sys")
-    def test_inproc_dll_reg_threading(self, _sys):
-        _sys.mock_add_spec(["frozen", "modules"])
-        _sys.frozen = "dll"
-        _sys.modules = sys.modules
+    @mock.patch.object(register, "_get_serverdll", return_value=SERVERDLL)
+    def test_inproc_reg_threading(self, get_serverdll):
         reg_clsid = GUID.create_new()
         reg_threading = "Both"
         reg_clsctx = comtypes.CLSCTX_INPROC_SERVER
@@ -525,9 +479,15 @@ class Cls:
         full_classname = f"{__name__}.Cls"
         expected = [
             (HKCR, clsid_sub, "", ""),
-            (HKCR, inproc_srv_sub, "", r"my\target\server.dll"),
+            (HKCR, inproc_srv_sub, "", self.SERVERDLL),
+            # 'PythonClass' and 'PythonPath' are not required for
+            # frozen inproc servers.  This may be bugs but they do
+            # not affect the server behavior.
             (HKCR, inproc_srv_sub, "PythonClass", full_classname),
             (HKCR, inproc_srv_sub, "PythonPath", os.path.dirname(__file__)),
             (HKCR, inproc_srv_sub, "ThreadingModel", reg_threading),
         ]
-        self.assertEqual(expected, list(RegistryEntries(Cls)))
+        self.assertEqual(
+            expected, list(RegistryEntries(Cls, frozen="dll", frozendllhandle=1234))
+        )
+        get_serverdll.assert_called_once_with(1234)