Skip to content

Fix: reopen async File passed to Process.exec and .run (win32)#15703

Merged
straight-shoota merged 5 commits intocrystal-lang:masterfrom
ysbaddaden:fix/win32-process-exec-and-run-with-async-files
Apr 29, 2025
Merged

Fix: reopen async File passed to Process.exec and .run (win32)#15703
straight-shoota merged 5 commits intocrystal-lang:masterfrom
ysbaddaden:fix/win32-process-exec-and-run-with-async-files

Conversation

@ysbaddaden
Copy link
Collaborator

@ysbaddaden ysbaddaden commented Apr 24, 2025

The arguments for input, output and error must be handles without FILE_FLAG_OVERLAPPED so we can't pass a File opened with blocking: false... but we can reopen the file from its #path using ReOpenFile as blocking.

This feels more like a hack than a proper solution. I initially tried ReOpenFile, but I couldn't get it to work, while the crude hack just worked 🤷

This is working as long as the file isn't closed, in which case we can just use the current file handle: we can pass an async closed handle (but can't reopen a closed file).

Extracted from #15685

@ysbaddaden ysbaddaden added platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:runtime labels Apr 24, 2025
@ysbaddaden ysbaddaden self-assigned this Apr 24, 2025
@ysbaddaden
Copy link
Collaborator Author

I tried ReOpenFile again. I did something wrong previously, like trying to reopen non-File handles, because now it appears to work correctly.

See e351e6c in #15685. Let's see if CI succeeds for the PoC.

@ysbaddaden ysbaddaden marked this pull request as draft April 24, 2025 13:27
@ysbaddaden

This comment was marked as resolved.

@ysbaddaden
Copy link
Collaborator Author

Updated to use ReOpenFile.

@ysbaddaden ysbaddaden marked this pull request as ready for review April 25, 2025 06:02
@straight-shoota straight-shoota added this to the 1.17.0 milestone Apr 25, 2025
@straight-shoota straight-shoota added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Apr 28, 2025
@straight-shoota straight-shoota merged commit bf63fc7 into crystal-lang:master Apr 29, 2025
35 checks passed
@ysbaddaden ysbaddaden deleted the fix/win32-process-exec-and-run-with-async-files branch April 29, 2025 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants