Skip to content

Conversation

@Mistuke
Copy link
Contributor

@Mistuke Mistuke commented May 23, 2022

There's a small bug in the modes that we create handles with when we make inheritable handles.

We create both the input and output handles with FILE_FLAG_OVERLAPPED.
This is OK as long as the program being called is also not using IOCP. For the majority of the program
usually called this is the case and so we never noticed the bug in testing.

However when calling an application which is also using overlapped IO or uses the kernel low level
async APIs then the spawned application will misbehave because the operation would unexpectedly
become asynchronous. This is the case with any program using Microsoft VC++'s stdlibs to read pipes.

The fix is to only mark handles with FILE_FLAG_OVERLAPPED if they are not being inherited. That is,
only make them asynchronous if we, the caller will be using them and not the callee.

Note that WINIO can handle synchronous handles as well, so even if some other part of the program
uses the synchronous handle it would still work correctly.

Fixes https://gitlab.haskell.org/ghc/ghc/-/issues/21610

/cc @bgamari

@snoyberg
Copy link
Collaborator

LGTM, though I can't claim I have a full understanding. Can you add a changelog entry about this? And does this warrant an immediate release, or should it be bundled with upcoming releases?

@Mistuke
Copy link
Contributor Author

Mistuke commented May 24, 2022

LGTM, though I can't claim I have a full understanding. Can you add a changelog entry about this?

Sure, will do!

And does this warrant an immediate release, or should it be bundled with upcoming releases?

I think that probably depends on if @mpickering thinks it can be included in the 9.4 release, otherwise no rush.

@Mistuke Mistuke force-pushed the wip/only-mark-non-inheritable-handles-as-overlapped branch from 97b0162 to cc148bf Compare May 27, 2022 08:41
@Mistuke
Copy link
Contributor Author

Mistuke commented May 27, 2022

@snoyberg changelog added. looks like there no need for an immediate release, just merging should be enough and I'll be able to close the GHC ticket. thanks!

Copy link
Collaborator

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@snoyberg snoyberg merged commit af0614c into haskell:master May 27, 2022
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