Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PythonService DoLogMessage raising fatal GIL lock error. #2426

Merged
merged 2 commits into from
Nov 14, 2024

Conversation

JacobNolan1
Copy link
Contributor

@JacobNolan1 JacobNolan1 commented Nov 13, 2024

Opening pull request for potential fix to #2155. @guentner suggested moving freeing of msg object in LogErrorMsg call out of PyBeginAllowThreads block.

On Python 3.12 services were crashing from a call to servicemanager.LogErrorMsg().
Found error occurred when tested on 3.12.2 and 3.12.7

By commenting out all calls to LogErrorMsg() the service crashes were fixed.

@guentner found this was being raised here PythonService.cpp

Fatal Python error: _PyMem_DebugFree: Python memory allocator called without holding the GIL

I've used the artifact build for Python3.12 and this has resolved my issue.

…g of variable out of Py Allow Threads block
@JacobNolan1 JacobNolan1 marked this pull request as ready for review November 13, 2024 06:30
Copy link
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! Do you mind adding a CHANGELOG entry for this?

@Avasam Avasam linked an issue Nov 13, 2024 that may be closed by this pull request
@Avasam
Copy link
Collaborator

Avasam commented Nov 13, 2024

I just did a pass looking at all PyWinObject_FreeWCHAR and couldn't find any other that was also immesiatly/obviously wrapped in *_ALLOW_THREADS. So this should be the only one 👍

@JacobNolan1
Copy link
Contributor Author

Thanks! Do you mind adding a CHANGELOG entry for this?

Hey Mark, no worries. I've updated the changelog.txt, is this fine?

Copy link
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 d618bd5 into mhammond:main Nov 14, 2024
30 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.

servicemanager LogInfoMsg and LogErrorMsg are crashing
3 participants