[server\register.py] Use WinDLL instead of windll + import improvement

[moi15moi]

Fix #735 for the server\register.py module

[junkmd]

@moi15moi

Thank you for taking on such a significant task.

In this repository, the CI pipeline does not run automatically for contributor pull requests until the first PR is merged and approved by a maintainer.
Additionally, if you include all the changes for #735 in a single PR, it may result in a large amount of code to review, which could delay the merging process.

And I noticed that some of the CI pipeline's automated tests are failing.
There might be some unexpected changes related to the server.

I recommend cherry-picking smaller, less impactful changes, such as the SHDeleteKey in server/register.py, and opening it as the first PR.
(Going forward, I may also ask you to split changes into several smaller PRs to keep the scope of each review manageable.)

[junkmd]

For the parts where oledll.foobar was originally used, I think it would be better to use OleDLL("foobar").
What do you think?

Also, since your first PR has not yet been approved and merged, my approval is required each time you push changes to trigger the CI pipeline.
As I mentioned earlier, I recommend opening PRs with small, easy-to-merge changes.

[moi15moi]

Going forward, I may also ask you to split changes into several smaller PRs to keep the scope of each review manageable

I agree that the PR has a lot of changes, but if you review it commit per commit, it shouldn't be unmanageable. Creating multiple PR will require you the same amount of work but simply on a longer period of time because I won't open all the PR in one shot.

If you still maintain your opinion, how do you expect me to divide the commit? One for comtypes/test, another one for the server and another one for the client?

For the parts where oledll.foobar was originally used, I think it would be better to use OleDLL("foobar").
What do you think?

I think it's better to only keep OleDLL or WinDLL to have more consistent code. Having both of them is pretty useless since I specify the argtypes and restype for each function.

I choosed WinDLL because I observate weird behaviour with OleDLL. For example, for this line, if I change windll.oleaut32.SysAllocStringLen to _oleaut32.SysAllocStringLen, I get this error when I run the tests:

File "C:\Users\moi15moi\Documents\GitHub\comtypes\comtypes\test\test_showevents.py", line 24, in comtypes.test.test_showevents.ShowEventsExamples.StdFont_ShowEvents
Failed example:
    font.Name = 'Arial'
Exception raised:
    Traceback (most recent call last):
      File "C:\Users\moi15moi\AppData\Local\Programs\Python\Python311\Lib\doctest.py", line 1353, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest comtypes.test.test_showevents.ShowEventsExamples.StdFont_ShowEvents[5]>", line 1, in <module>
        font.Name = 'Arial'
        ^^^^^^^^^
      File "C:\Users\moi15moi\Documents\GitHub\comtypes\comtypes\_post_coinit\_cominterface_meta_patcher.py", line 33, in __setattr__
        object.__setattr__(self, self.__map_case__.get(name.lower(), name), value)
      File "C:\Users\moi15moi\Documents\GitHub\comtypes\comtypes\_memberspec.py", line 514, in fset
        return obj.Invoke(memid, value, _invkind=invkind)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "C:\Users\moi15moi\Documents\GitHub\comtypes\comtypes\automation.py", line 879, in Invoke
        dp = self.__make_dp(_invkind, *args)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "C:\Users\moi15moi\Documents\GitHub\comtypes\comtypes\automation.py", line 854, in __make_dp
        array[i].value = a
        ^^^^^^^^^^^^^^
      File "C:\Users\moi15moi\Documents\GitHub\comtypes\comtypes\automation.py", line 290, in _set_value
        self._.c_void_p = _SysAllocStringLen(value, len(value))
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "_ctypes/callproc.c", line 1000, in GetResult
    OverflowError: Python int too large to convert to C long

I am unsure why this issue only occurs with OleDLL, but it seems very strange to me. According to the ctypes documentation, the only difference between OleDLL and WinDLL is the default restype: HRESULT for OleDLL and int for WinDLL. However, there must be a more significant underlying difference, as WinDLL does not fail under the same circumstances.

Also, since your first PR has not yet been approved and merged, my approval is required each time you push changes to trigger the CI pipeline.

I simply created a PR on my fork. It is easier for me since it automatically run the test.
But, I have a weird problem.
On my fork, I sometimes I get memory leaks even on the main branch as you can see here: https://github.com/moi15moi/comtypes/actions/runs/12738567411/job/35501093520
I don't understand why it doesn't happen on the main branch of this repos.

[junkmd]

I agree that the PR has a lot of changes, but if you review it commit per commit, it shouldn't be unmanageable. Creating multiple PR will require you the same amount of work but simply on a longer period of time because I won't open all the PR in one shot.

Nevertheless, there is value in splitting changes into multiple PRs one by one.

Consider a case where changes to A.py in a PR can be merged immediately, but changes to B.py require further discussion. If the changes to A.py are in a separate PR, it reduces the amount of discussion required for each PR. Furthermore, by the time discussions about B.py are concluded, conflicts may arise when attempting to merge A.py. At that point, additional effort will be needed to review whether the conflict resolution is reasonable.

For example, "Small CLs" in "Google Engineering Practices Documentation" shows great practices about these.
https://google.github.io/eng-practices/review/developer/small-cls.html

If you still maintain your opinion, how do you expect me to divide the commit? One for comtypes/test, another one for the server and another one for the client?

When reviewing a PR related to a similar replacement task (#657) in the past, I noticed that a single PR with changes to the existing codebase spanning multiple modules and exceeding about 30 lines placed a cognitive burden on reviewers.¹

I propose splitting PRs (or issues) based on the following priorities:

• Replace windll.foobar with Windll("foobar").
• Ensure the above changes do not exceed about 30 lines across multiple modules (when long lists are wrapped by ruff, count each list as one line).
• Changes within a single module can exceed 30 lines, but splitting them further would still be appreciated.
• Investigate issues related replacing oledll.foobar with OleDLL("foobar").

The basis for this prioritization is that this type of task does not require making extensive code changes all at once to avoid breaking something or ensuring functionality.

---

Footnotes

1. In contrast, for cases like Implement record pointers as method parameters of a Dispatch Interface #535, where entirely new conditional branches or test double codebases were added, even hundreds of lines did not impose a significant cognitive load. This difference is likely due to how easily the behavioral changes in production code can be anticipated and how straightforward it is to estimate the attention required for backward compatibility. That said, it still took over three weeks for the PR to move from open to merged. ↩

[junkmd]

I am unsure why this issue only occurs with OleDLL, but it seems very strange to me. According to the ctypes documentation, the only difference between OleDLL and WinDLL is the default restype: HRESULT for OleDLL and int for WinDLL. However, there must be a more significant underlying difference, as WinDLL does not fail under the same circumstances.

Indeed, this is puzzling.
I read the ctypes source code, but from the Python code alone, I couldn't determine the cause of this discrepancy. The issue might lie in the C extensions or cffi.

That said, despite the issue described in #735, oledll.foobar.SomeFunction is still functioning. Moreover, this project will not break just because all the functions aren’t replaced at once. On the contrary, I’m more concerned about missing something important when making large-scale changes.

I think it’s reasonable to start by just only replacing windll with WinDLL, as this change has proven to be reasonable and free of runtime problems, and then proceed with investigating oledll afterward.

[junkmd]

Also, since your first PR has not yet been approved and merged, my approval is required each time you push changes to trigger the CI pipeline.

I simply created a PR on my fork. It is easier for me since it automatically run the test.
But, I have a weird problem.
On my fork, I sometimes I get memory leaks even on the main branch as you can see here: https://github.com/moi15moi/comtypes/actions/runs/12738567411/job/35501093520
I don't understand why it doesn't happen on the main branch of this repos.

I believe this memory leak is a one-time glitch.
Since this project depends on the state of COM in the runtime environment, I suspect it occurs when the CI virtual environment is in an unstable condition.
While stabilizing this might be necessary, it likely falls under the scope of cpython.

By re-running the failed jobs multiple times, it is possible to get CI results where this error does not occur.

[junkmd]

I recommend cherry-picking smaller, less impactful changes, such as the SHDeleteKey in server/register.py, and opening it as the first PR.

This perspective remains unchanged.

[moi15moi]

I recommend cherry-picking smaller, less impactful changes, such as the SHDeleteKey in server/register.py, and opening it as the first PR.

This perspective remains unchanged.

Done. Let me know what you think.

[junkmd]

Since the CI has passed, it has been confirmed that there is no issue with the WinDLL change itself.
Now, it's just a matter of adjusting the scope of the changes.

[junkmd]

I think it's fine to leave os.path.foobar as is. For #735, let's limit it to changes in the import and reference methods from ctypes and comtypes, and move the rest to a separate scope.

[moi15moi]

Done. Let me know what you think.

[junkmd]

Thank you!

The PR written "Fix #735 ..." on the Kanban might automatically close by GitHub's "Linked Issue" feature when it is merged.
https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue

Of course, I understand that the scope of #735 isn't limited to just the changes in server/register.py.
If the issue does get closed, I’ll reopen it manually.