gh-105436: the environment block should end with two zero-valued wchar_t values#105495
gh-105436: the environment block should end with two zero-valued wchar_t values#105495zooba merged 13 commits intopython:mainfrom
Conversation
A Unicode environment block is terminated by four zero bytes
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
Modules/_winapi.c
Outdated
| /* A Unicode environment block is terminated by four zero bytes: | ||
| two for the last string, two more to terminate the block. */ | ||
| if (envsize == 0) { | ||
| buffer = PyMem_NEW(wchar_t, 2); | ||
| p = buffer; | ||
| *p++ = L'\0'; | ||
| *p++ = L'\0'; | ||
| goto error; | ||
| } |
There was a problem hiding this comment.
Using goto error is misleading. I suggest changing the function's exit label name to "cleanup".
Using PyMem_Calloc() avoids the need to manually zero the buffer for the null characters. Also, make sure to raise a MemoryError exception if allocation fails.
| /* A Unicode environment block is terminated by four zero bytes: | |
| two for the last string, two more to terminate the block. */ | |
| if (envsize == 0) { | |
| buffer = PyMem_NEW(wchar_t, 2); | |
| p = buffer; | |
| *p++ = L'\0'; | |
| *p++ = L'\0'; | |
| goto error; | |
| } | |
| if (envsize == 0) { | |
| // A environment block must be terminated by two null characters -- | |
| // one for the last string and one for the block. | |
| buffer = PyMem_Calloc(2, sizeof(wchar_t)); | |
| if (!buffer) { | |
| PyErr_NoMemory(); | |
| } | |
| goto cleanup; | |
| } |
… the last string and one for the block.
…to issues_105436
Misc/NEWS.d/next/Windows/2023-06-08-11-30-17.gh-issue-105436.1qlDxw.rst
Outdated
Show resolved
Hide resolved
…qlDxw.rst Co-authored-by: Eryk Sun <eryksun@gmail.com>
|
I can't reproduce the problem in the test. |
|
I guess the new test has exposed a leak on Ubuntu that no other tests trigger. Maybe make the new test Windows only and tag it with this issue? No reason to block your change here - it's definitely not the cause of that leak. |
…_t values (pythonGH-105495) (cherry picked from commit 4f7d3b6) Co-authored-by: Dora203 <66343334+sku2000@users.noreply.github.com>
|
GH-105700 is a backport of this pull request to the 3.12 branch. |
|
GH-105701 is a backport of this pull request to the 3.11 branch. |
…_t values (pythonGH-105495) (cherry picked from commit 4f7d3b6) Co-authored-by: Dora203 <66343334+sku2000@users.noreply.github.com>
|
|
The child process failing at startup with |
|
Are we copying |
|
There's already a >>> sys.version_info[:2]
(3, 10)
>>> subprocess.call(sys.executable, env={'a':'b'})
Fatal Python error: _Py_HashRandomization_Init: failed to get random numbers to initialize Python
Python runtime state: preinitializedThe The bot that's failing is an ARM64 system. I don't know whether process initialization under ARM64 depends on |
|
The ARM64 bot has a slightly different setup compared to the rest of our machines, because it's doing a cross-compile and then running elsewhere. So there might be something critical in But you're right, the only important part is the OSError. I've reopened the bug, let's move discussion there. |
In the CreateProcessW document, the environment block should end with two zero-valued wchar_t values(i.e. four zero bytes).
subprocess.run(..., env={})broken on Windows #105436