Skip to content

Conversation

@pyrmont
Copy link
Contributor

@pyrmont pyrmont commented Dec 15, 2025

While working on Churlish (a wrapper around the curl CLI utility), I noticed intermittent errors while using os/spawn on Windows. After further investigation, I believe the issue is that I was calling os/spawn with an effectively empty1 environment and that this was causing CreateProcess to occasionally fail due to a bug in os_execute_env.

The issue is that an empty environment block needs to be double-NULL terminated as described in more detail in Microsoft's documentation:

Note that an ANSI environment block is terminated by two zero bytes: one for the last string, one more to terminate the block. A Unicode environment block is terminated by four zero bytes: two for the last string, two more to terminate the block.

I suspect that the error was intermittent because sometimes the memory being accessed happened to have a second NULL byte after the first. This was not guaranteed to be the case.

This commit adds an extra NULL byte if the environment is empty. It also adds a test.

Footnotes

  1. I was calling os/spawn with a struct that mapped :in, :out and :err to pipes. I mistakenly thought that this meant I needed to use :pe as the second argument to os/spawn. Because the struct contained no other values, it was empty for the purposes of os_execute_env.

@pyrmont pyrmont marked this pull request as draft December 15, 2025 01:25
@pyrmont
Copy link
Contributor Author

pyrmont commented Dec 15, 2025

For future reference, binaries that are compiled with MinGW can fail on Windows (not WINE) when launched with os/execute (or os/spawn) because the absence of PATH causes system DLLs not to load. As best as I can tell, this is a MinGW-specific issue and results in error 0xC0000135 (exit code -1073741515). The test case checks if running MinGW on Windows and, only if that is the case, permits that exit code.

This is obviously not a great solution but I think it's the most reasonable approach. All the alternatives I could think of (e.g. silently adding PATH to the 'empty' environment when running in MinGW-compiled Janet, always panicking even though there might not be a problem) seemed worse.

@pyrmont pyrmont marked this pull request as ready for review December 15, 2025 03:10
@bakpakin
Copy link
Member

Looks good, that is a subtle issue. A missing PATH environment variable is a recipe for all sorts of issues, so I wouldn't worry too much about that failure mode

@bakpakin bakpakin merged commit 7d672f4 into janet-lang:master Dec 15, 2025
13 checks passed
@pyrmont pyrmont deleted the bugfix.windows-empty-env branch December 15, 2025 04:23
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.

2 participants