Os.exec win32 #1089

coreyapowell · 2025-05-30T23:06:05Z

I think I have this figured out, minimized changes for win32 functionality and testing of os.exec.
someone should test on a real linux machine .. and you're doing mac?
I had to change several #ifdef (wasi) lines. I wasn't expecting to just change the codebase. I'm sorry about going back and forth, but someone changed permissions while I was in the middle of uploading files and it's created a huge mess that I can't seem to fix!
downloaded compiled and tests run with MING64/Clang64 Msys2, Debian on WSL, MSVC 2022, Clang for MSVC

I put this back like it was. Changes will be split into smaller parts and I've gotten confused when permissions went back and forth

Update test_std.js

getting back to original codebase

tests on win32: os.exec os.pipe (fd version) os.kill os.watchpid maintains test on linux: os.exec os.kill os.waitpid os.watchpid os.pipe

adds os_exec, os_pipe, build_ms_envp, os_watchpid (2 versions) os_kill

just modified comments and changed name. I do plan to test watchpid on non-win32.

managed to also verify execution of .exe, return values of exe, and stderr handle. verifies watchpid in non-win32 for compatible scripting. the only non-compatible issue left I see is that getline() pulls from windows needing \r to be trimmed from <eol>

jscheck issue fixed. I did move the functions around to minimize code differences. env in win32 is one big zero-delimited buffer; I now double its size and copy on overflow.

coreyapowell · 2025-05-31T19:06:33Z

os.exec branch is updated and tested.
I'm still learning! I did figure out how to do jscheck. is there a one-stop place for me to run those tests? I couldn't find a way to do that in github myself. I ran it from WSL.
I've left comments on this os.watchpid in the test_os_exec.js and in libc.c because I didn't know how to handle adding to the documentation. If you'd prefer that, I can do that, but it's not any normal c or linux function. it would require explanation. Everything else I've added is straight forward.

it's semantics. I just tested it. my compiler is not liking size_t addition to char* and for some reason even code editor seems to think it's typecasting crazy. so I'm comparing references to keep it clean enough looking.

needed to free the memory when I re-cache.

tested again ... I had comments and printf functions to remove. I just keep reviewing and expanding my test environment.

bnoordhuis

There are a bunch of style issues, like inconsistent whitespace, non-idiomatic (for this project) if/else style, etc. Can you fix those first?

Rule of thumb: new code should look like the existing code that surrounds it.

coreyapowell · 2025-06-02T04:05:46Z

Sure.
I'm trying to understand how you guys collaborate as independent users. I want whoever is handling the windows portion to at least have a heads up. It's as 'drop in replacement' as I could manage. I just expected someone might have some feedback.

I don't want to trample over everyone else's projects. In fact, I believe Mr Bellard likely ran into the same issues (that windows doesn't play nice with FDs and PIDs) and just skipped all of this. The quickjs-ng team added worker pipes for win32 (in quickjs-libc.c), but they use HND types in a piece of code that users won't see in the script. The quickjs documentation, the current std and os module, the c standard, and all of linux use and prefer PIDs and FDs. I haven't had a chance to test things like passing the FD down a pipe to another .exe or to a worker thread. Passing out HNDs in script would guarantee that flexibility, but it would be a documentation nightmare. So this works, it does what I need it to do as a scripting tool, and it function simply enough.

I've created one replacement function that can be used in linux and windows without testing the environment, but the user needs to understand what that function does. Windows doesn't have waitpid. Do I just leave it in the tests.js folder and let them figure it out? Leave my comments above the c code and test.js code? Or put a comment in the documentation section for watchpid next to waitpid? Is everyone okay with the way I split those functions up in test_os_exec.js? I am just trying to make the use clear. Is everyone happy with the way I've left that?

I appreciate the feedback so far, I don't expect anyone to take up all their time on this. I just know what any windows user or admin needs, and I'm trying to complete it.

removed the majority of flags. Only left flags that seemed necessary. I had a combined js_os_pipe with several #ifdefs inside to separate the OSs. I later restored the non-WIN32 version so that it would not be out of order from the code before. The logic was that a) the export list is maintained in the order of the function calls. and b) the code compare was less of a mess to keep that chunk intact. I realized there were useless #ifdefs and removed them. I did retest both WIN32 and GNU.

I think I got everything! mostly comments and reordering of if statements. I'm not the only programmer in the world to put my then branches on the same line following an if statement. But I'm genX, I used 640x480 CRTs. It looks clearer, I do admit that. But this is definitely a 21st century practice, in the old days we couldn't afford the Tabs or the printer paper.

nothing functional, just tweaking the comments. I'm not happy leaving this undocumented function in the wild. and it's trivial! but there it is! it at least deserved 2 lines of description. It is more concise now .... just like Bellard would have wanted ... tidy and such.

coreyapowell · 2025-06-02T08:48:39Z

Yes, I found a few things that needed cleaning up. Hopefully that's sufficient.
Until this functionality is there, I'm having to edit these long batch files to run all my build configs every time I make a change, download, retool, test, repeat. This IS the tool to automate all of that. So I don't know if there's a merge tool in github that's going to take my 2 sets of changes and combine them. But if not, I'm going to have to merge these changes now, to build the tool, to retool the build, to build the tests that test the build.

past-due · 2025-06-02T14:50:53Z

quickjs-libc.c

+    int pid;
+    HANDLE ph;
+    int block = 0, ret; 
+    DWORD options = 0, flags = PROCESS_QUERY_INFORMATION | SYNCHRONIZE;


This probably should be:

Suggested change

DWORD options = 0, flags = PROCESS_QUERY_INFORMATION | SYNCHRONIZE;

DWORD options = 0, flags = PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE;

tested and works fine like that. I'll put that in. There's better chances of it not tripping security checkpoints in someone else's random section of code with LTD INFO.

apparently you close PID HNDS. Unlike FDs to HND casts.

I've tested this a good bit over here. I think the handle numbers are recycling, just not immediately. Everything works.

PROCESS_QUERY_LIMITED_INFORMATION

CloseHandle after OpenProcess. The documentation wasn't so clear, but this works as it should, and I read that it wastes resources not to. Don't close HNDs to FDs, let them get closed by the user. But close HNDs from PIDs. Besides, this is a kill and an exit return value. It's stopped!

coreyapowell · 2025-06-02T17:52:18Z

If you guys want to take some time reviewing this, I'm trying to pass these handles down pipes through stdin see if the child permissions work.

past-due · 2025-06-02T18:27:28Z

If you guys want to take some time reviewing this, I'm trying to pass these handles down pipes through stdin see if the child permissions work.

You may want to take a look at: https://learn.microsoft.com/en-us/windows/win32/sysinfo/handle-inheritance

coreyapowell · 2025-06-02T19:20:22Z

thanks!
It works great as a stdio handle, but i've tried everything. I know you can pass multiple pipes and sockets to the child, but I think the answer to what I'm thinking is "named pipes". There's no other permissions I haven't tried and I get no difference. The sharing is on the process stdio. I thinkk this does what it's supposed to do.
Now I did block the stdio from passing from the child back through the parent to the console. That was a flag. I assume that's how the linux version works. I assume this is Canon.

coreyapowell · 2025-06-03T11:52:02Z

run-test262: unknown option: -q
the script looks at the executable name and path, which on my end was qjs.exe. it uses that to execute another qjs(.exe) in the test. I just needed the path of the executable, because it's not necessarily the CWD.
I see now how you're running test262. I've been asking this, because I can't get 262 to run with the usage example. I'll work on that. That's actually helpful information. I don't know if someone else does the Cmake, but the MINGW builds are different between your CMake and Bellard. Does one person make the CMake script and the other person make the documentation online? Nothing is clear, and 262 just says I have the wrong parameters.

I think I just need to fix the test script, to truncate the name from the path and append "qjs". But I have to get test262 to actually run to fully verify that.

coreyapowell · 2025-06-03T12:06:39Z

also I'm working on this and remembering, do you want to just combine this back into test_std.js? or keep this file separate? it's becoming sort of misplaced as test_std does some os module tests.

recombined the functions tested against: (linux) true exit 2 bad command -> 127 good command ->0 watchpid blocking environment inherited pipe waitpid blocking watchpid nonblocking pid running kill watchpid nonblocking pid ended waitpid WNOHANG I have uncovered that linux echoes to the stdout and I am going to revert my flags for win32, but that change isn't tested yet in this file.

don't sleep on blocking, moved comment too

past-due · 2025-06-03T18:06:31Z

For things like js_os_exec, it might be better to refactor it into separate functions (for Windows, vs the existing for others), instead of those huge blocks of #ifdefs with very different implementations that make it much harder to read through. Just my two cents.

"shell commands come through" split the os_exec function back in half between the 2 versions. fixed the way handles inherit and text buffers FROM SHELLS! text does not buffer from system commands, especially bad ones. Error messages are not the same. verifying 7 permutations are the same between the two OSs

7 permutations of direct commands and shell commands firing error codes and text through console. shells inherit stdio unless you pass a pipe. this is exec, bad programs don't execute. echo doesn't execute! there's text output now. it may be to set a flag to disable text output for testing, or else execute a script and verify the pipe text.

coreyapowell · 2025-06-03T21:46:18Z

Done! functions separated between the operating systems and code cleaned up.

There were bugs. The test script now goes through 7 permutations to show and explain that echo is not a process. Besides the return values being different and the text from shells, the behavior of those 7 permutations is identical. The error code for a bad program is not the same as cmd /c not finding a command.

Shells inherit the handles by default if you don't pass a pipe. That's the way it was set up. But that's why the text program outputs "bad" and "shell commands come through". It's a change to the way test scripts work, but it needed verifying.

Now, these 2 things can be changed! I started creating a flag that is not in the linux code when passing the stdio handles to CreateProcess. I think that shutting off the pipes and just reading the exit status from processes would be the more desired option, so a flag should be implemented. I was calling it 'inherit', and we can do this either way, because we pass stdio 1,2,3 by default. I think it should not be by default, silence should be default. It's less intrusive than creating 2 pipes to silence the output.

I think a flag in test_os_exec.js for demonstrating the output text is more fitting. But now you have the option to execute "qjs test_os_exec.js" from test_std.js and just silently assert that shell text was there (similar to how you have test_worker and test_worker_module but only execute the master, the other is the slave).

expanded 2 more functions and improved the test on file and inherit. nothing is yet tested on cwd.

added inherit flag it became more obvious that stdio should default to silent. set the inherit flag or pass one pipe, and the child will inherit the stdio of the parent (except the pipe you set). I believe this includes stdin!

coreyapowell · 2025-06-04T01:12:57Z

silent by default, set inherit: true to pass stdio to the child, seems like the right thing to do.
linux version modified to do the same. I have a backup if that's not what you want.

test_os_exec_child.js - executes the 8 basic permutative shell tests and verifies that only inherited handle text passes through stdout and stderr. test_os_exec.js - now builds the test folder path and executes test_os_exec_child.js; verifies that stderr sees 'bad' and stdout only sees the proper echo string. tests.conf will need update for all of this. It comes to my attention there is no return value when doing asynchronous shell commands.

test_os_exec_child.js added to exclusions. there's no other way to silently verify stdout and stderr inheritance.

amended child execution to test cwd parameter of os.exec

coreyapowell · 2025-06-04T23:11:45Z

updated scripts to work with test262. Verifying 8 permutations is offloaded to test_os_exec_child.js. this verifies:

inherited stdio, echo text passes through stdout,
'bad' shell text passes through stderr.
using the cwd parameter to locate the child script.
using the word 'child' instead of 'slave', I've been programming hardware for 30 years and the political landscape has changed.

I was building the script path, but I needed to test cwd. test262 is usually called from the make folder. manually I'm going into tests/ and calling from there so that assert.js is there. now I look and see if 'tests' is the end of the path, if not I insert it, and I use 'cwd: dir,' If someone executes test262 from a build folder, I don't know how they get assert.js, but this will then break.

I need the cwd, and it needs testing. detects if you're in 'build' detects if you're in 'scripts' otherwise adds 'scripts' then it builds the cwd to use and test.

test_os_exec moved to other js

coreyapowell · 2025-06-05T20:22:51Z

I need some eyes on this in case it won't work for you.
I have to test cwd on os_exec, so i call os.getcwd(). But run-test262 will likely run from build or from the root project directory, so I pop 'build', and I push 'tests', and I join. Then I execute the child script from there, testing the cwd parameter.

I assume everything is fine, I can run-test262 without complaint.
But I personally don't make 5 full folders from github, I make 5 build folders. So there's only room for the Makefile defaults. I'll do one win32 version and a WSL version, but I manually run the test scripts otherwise.
I can't think of a better way, but someone with a different build has to set this the way they want.

also, after sleeping on this, I've added a 'inherit' flag to make stdio pass through from the child to the console executing qjs. After I've slept on it and gotten familiar with how fork and execvpe function, I'm not happy with it. Inherited is default with a fork, it's not with Windows, but it should default to the linux standard. I think it should be a 'silent' flag.

coreyapowell added 9 commits May 29, 2025 16:02

Add files via upload

6ee553f

Add files via upload

f323678

Add files via upload

7774b71

Add files via upload

ba9e4c4

Update test_std.js

3d43bdb

I put this back like it was. Changes will be split into smaller parts and I've gotten confused when permissions went back and forth

Merge pull request #2 from coreyapowell/backto_original

60c56b1

Update test_std.js

Delete test_std.js

a5395d3

getting back to original codebase

adding test script for win32 os.exec

78fccd9

tests on win32: os.exec os.pipe (fd version) os.kill os.watchpid maintains test on linux: os.exec os.kill os.waitpid os.watchpid os.pipe

os_exec

a1e3c0e

adds os_exec, os_pipe, build_ms_envp, os_watchpid (2 versions) os_kill

coreyapowell marked this pull request as ready for review May 31, 2025 02:05

coreyapowell added 3 commits May 31, 2025 13:32

Update and rename test_win_exec.js to test_os_exec.js

98608a0

just modified comments and changed name. I do plan to test watchpid on non-win32.

Update test_os_exec.js

9750c9d

managed to also verify execution of .exe, return values of exe, and stderr handle. verifies watchpid in non-win32 for compatible scripting. the only non-compatible issue left I see is that getline() pulls from windows needing \r to be trimmed from <eol>

Update quickjs-libc.c

f9dbd9c

jscheck issue fixed. I did move the functions around to minimize code differences. env in win32 is one big zero-delimited buffer; I now double its size and copy on overflow.

coreyapowell added 3 commits May 31, 2025 15:12

Update quickjs-libc.c

dc052ee

it's semantics. I just tested it. my compiler is not liking size_t addition to char* and for some reason even code editor seems to think it's typecasting crazy. so I'm comparing references to keep it clean enough looking.

Update quickjs-libc.c

6c0db1a

needed to free the memory when I re-cache.

Update quickjs-libc.c

08183f7

tested again ... I had comments and printf functions to remove. I just keep reviewing and expanding my test environment.

bnoordhuis reviewed Jun 1, 2025

View reviewed changes

coreyapowell added 3 commits June 2, 2025 02:55

past-due reviewed Jun 2, 2025

View reviewed changes

coreyapowell added 2 commits June 2, 2025 11:42

Update quickjs-libc.c

688707e

PROCESS_QUERY_LIMITED_INFORMATION

coreyapowell added 2 commits June 3, 2025 10:22

Update test_os_exec.js

09c6620

don't sleep on blocking, moved comment too

coreyapowell added 2 commits June 3, 2025 17:31

coreyapowell added 2 commits June 3, 2025 20:08

Update test_os_exec.js

4b317d3

expanded 2 more functions and improved the test on file and inherit. nothing is yet tested on cwd.

Update quickjs-libc.c

229918d

added inherit flag it became more obvious that stdio should default to silent. set the inherit flag or pass one pipe, and the child will inherit the stdio of the parent (except the pipe you set). I believe this includes stdin!

coreyapowell added 3 commits June 4, 2025 18:43

Update tests.conf

d565213

test_os_exec_child.js added to exclusions. there's no other way to silently verify stdout and stderr inheritance.

Update test_os_exec.js

2d859c6

amended child execution to test cwd parameter of os.exec

coreyapowell added 3 commits June 5, 2025 15:09

should work with test262

9f47581

I need the cwd, and it needs testing. detects if you're in 'build' detects if you're in 'scripts' otherwise adds 'scripts' then it builds the cwd to use and test.

removed test_os_exec

2e7cab6

test_os_exec moved to other js

	DWORD options = 0, flags = PROCESS_QUERY_INFORMATION \| SYNCHRONIZE;
	DWORD options = 0, flags = PROCESS_QUERY_LIMITED_INFORMATION \| SYNCHRONIZE;

Os.exec win32 #1089

Are you sure you want to change the base?

Os.exec win32 #1089

Uh oh!

Conversation

coreyapowell commented May 30, 2025

Uh oh!

coreyapowell commented May 31, 2025

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

coreyapowell commented Jun 2, 2025

Uh oh!

coreyapowell commented Jun 2, 2025

Uh oh!

past-due Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

coreyapowell Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

coreyapowell Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

coreyapowell Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

coreyapowell commented Jun 2, 2025

Uh oh!

past-due commented Jun 2, 2025

Uh oh!

coreyapowell commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coreyapowell commented Jun 3, 2025

Uh oh!

coreyapowell commented Jun 3, 2025

Uh oh!

past-due commented Jun 3, 2025

Uh oh!

coreyapowell commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coreyapowell commented Jun 4, 2025

Uh oh!

coreyapowell commented Jun 4, 2025

Uh oh!

coreyapowell commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

coreyapowell commented Jun 2, 2025 •

edited

Loading

coreyapowell commented Jun 3, 2025 •

edited

Loading

coreyapowell commented Jun 5, 2025 •

edited

Loading