Skip to content

Release GIL around blocking Win32 API calls in native functions#2732

Merged
mhammond merged 3 commits intomhammond:mainfrom
staer:fix/release-gil-blocking-calls
Apr 21, 2026
Merged

Release GIL around blocking Win32 API calls in native functions#2732
mhammond merged 3 commits intomhammond:mainfrom
staer:fix/release-gil-blocking-calls

Conversation

@staer
Copy link
Copy Markdown
Contributor

@staer staer commented Apr 18, 2026

We noticed in our project that several Win32 API calls were blocking the main thread for 100ms–2s+ each, preventing our asyncio event loop from making progress. After investigation, we found that a number of hand-written native functions call Win32 APIs without releasing the GIL. None of these functions access Python objects during the syscall, so holding the GIL is probably unnecessary.

Copy link
Copy Markdown
Owner

@mhammond mhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to request the formatting changes. Updating this to also handle #2732 would be fantastic.

Comment thread win32/src/win32net/win32netuser.cpp Outdated

err = NetUserEnum(szServer, level, filter, &buf, dwPrefLen, &numRead, &totalEntries, &resumeHandle);
if (err != 0 && err != ERROR_MORE_DATA) {
Py_BEGIN_ALLOW_THREADS err =
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I see how you copied the existing style, but sadly that "style" came from an erroneous auto-format some years ago, and I'm trying to fix them as we touch them. Could you please change these to taking a semi-colon and newlines after the macros? eg, like this code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do!

Copy link
Copy Markdown
Collaborator

@Avasam Avasam Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an option to handle this in clang-format

But adding a trailing ; also works since an empty statement after a brace still results in correct code.

Pros&cons can be discussed on that PR. For here it's sufficient to add ;

@Avasam
Copy link
Copy Markdown
Collaborator

Avasam commented Apr 20, 2026

Sorry to request the formatting changes. Updating this to also handle #2732 would be fantastic.

Did you mean #2731 ?

@staer
Copy link
Copy Markdown
Contributor Author

staer commented Apr 20, 2026

Fixed formatting, added #2731 to the list of calls changed and updated error handling a little bit. The 3.13, arm64 failure I think is just a test flake not related to this change.

@Avasam
Copy link
Copy Markdown
Collaborator

Avasam commented Apr 20, 2026

The 3.13, arm64 failure I think is just a test flake not related to this change.

Yeah that's #2203

@Avasam Avasam linked an issue Apr 20, 2026 that may be closed by this pull request
@Avasam Avasam requested a review from mhammond April 20, 2026 18:07
Copy link
Copy Markdown
Owner

@mhammond mhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@mhammond mhammond merged commit b5f7e74 into mhammond:main Apr 21, 2026
53 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CredUIPromptForCredentials blocks background threads

3 participants