Skip to content

os: support runtime poller with os.File on Windows #19098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ianlancetaylor opened this issue Feb 15, 2017 · 22 comments
Open

os: support runtime poller with os.File on Windows #19098

ianlancetaylor opened this issue Feb 15, 2017 · 22 comments
Labels
ExpertNeeded FixPending Issues that have a fix which has not yet been reviewed or submitted. OS-Windows Thinking
Milestone

Comments

@ianlancetaylor
Copy link
Member

In order to use the runtime poller with os.File on Windows, the file or pipe will have to be opened with FILE_FLAG_OVERLAPPED. We shouldn't do that unconditionally unless we can remove it if the program calls the Fd method. Programs currently expect the Fd method to return a handle that uses ordinary synchronous I/O.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/36800 mentions this issue.

@rasky
Copy link
Member

rasky commented Feb 15, 2017

ReOpenFile() can do just that, adding/removing FILE_FLAG_OVERLAPPED (https://msdn.microsoft.com/en-us/library/aa365497(VS.85).aspx)

gopherbot pushed a commit that referenced this issue Feb 15, 2017
This changes the os package to use the runtime poller for file I/O
where possible. When a system call blocks on a pollable descriptor,
the goroutine will be blocked on the poller but the thread will be
released to run other goroutines. When using a non-pollable
descriptor, the os package will continue to use thread-blocking system
calls as before.

For example, on GNU/Linux, the runtime poller uses epoll. epoll does
not support ordinary disk files, so they will continue to use blocking
I/O as before. The poller will be used for pipes.

Since this means that the poller is used for many more programs, this
modifies the runtime to only block waiting for the poller if there is
some goroutine that is waiting on the poller. Otherwise, there is no
point, as the poller will never make any goroutine ready. This
preserves the runtime's current simple deadlock detection.

This seems to crash FreeBSD systems, so it is disabled on FreeBSD.
This is issue 19093.

Using the poller on Windows requires opening the file with
FILE_FLAG_OVERLAPPED. We should only do that if we can remove that
flag if the program calls the Fd method. This is issue 19098.

Update #6817.
Update #7903.
Update #15021.
Update #18507.
Update #19093.
Update #19098.

Change-Id: Ia5197dcefa7c6fbcca97d19a6f8621b2abcbb1fe
Reviewed-on: https://go-review.googlesource.com/36800
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
@polarina
Copy link
Contributor

polarina commented Feb 15, 2017

Overlapped reads on Windows may run synchronously under some conditions. Files using NTFS compression can not be read asynchronously, for example.

See https://support.microsoft.com/en-us/help/156932/asynchronous-disk-i-o-appears-as-synchronous-on-windows

@ianlancetaylor
Copy link
Member Author

Thanks. For our purposes it doesn't matter if I/O is sometimes synchronous. The effect will be that the I/O operation will tie up a thread. That is not ideal, but it is no different than any other blocking syscall.

@jstarks
Copy link

jstarks commented May 11, 2017

There's no way to flip the bit on an existing file handle, and ReOpenFile will only work properly for real local files, only if the file does not have the delete on close bit set (I think), and only if they have been opened with compatible sharing flags. And of course Fd() uintptr does not provide a mechanism to return an error if something goes wrong.

I'm afraid this may be impossible to achieve with the current Windows API.

This is probably not such a shame for normal files opened for cached IO, although recent versions of Windows do perform cached reads asynchronously in some cases. But it certainly is limiting for named pipes, files opened with FILE_FLAG_NO_BUFFERING, and other Windows file handle types that reliably perform IO asynchronously.

Maybe this is something that we can improve in Windows. But even if we do, I think for Windows there would still be a lot of value in providing direct access to the runtime poller. There are several APIs that take OVERLAPPED structures that don't fit into os.File (ConnectNamedPipe, MergeVirtualDisk, DeviceIoControl, etc.), and it would be ideal to be able to use the existing poller to avoid blocking threads on these APIs.

That seems at odds with some of the discussion in #18507, unfortunately.

@qiulaidongfeng
Copy link
Member

CL has been merged, is it still necessary to open this issue?

@qiulaidongfeng
Copy link
Member

cc @golang/windows
This have OS-Windows label , CL has been merged. Is there anything else to be done here?

@crvv
Copy link
Contributor

crvv commented Jan 11, 2024

The merged CL(36800) updated this issue. It didn't fix this issue.
According to the discussion above. This issue can't be fixed because of the Fd method.
My personal opinion is that Fd usages should be replaced by SyscallConn. And Fd can be deprecated and removed.

@qmuntal
Copy link
Member

qmuntal commented Mar 24, 2025

We might not be able to change os.OpenFile to return a handle opened with FILE_FLAG_OVERLAPPED, but we could improve our os.File overlapped (aka async) I/O support for user-provided handles (via os.NewFile). That would certainly be useful for many.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/660495 mentions this issue: internal/poll: always use SetFileCompletionNotificationModes on non-socket handles

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/660496 mentions this issue: internal/poll,net: set SIO_UDP_CONNRESET in net

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/660595 mentions this issue: internal/poll: support async file operations on Windows

gopherbot pushed a commit that referenced this issue Mar 25, 2025
…ocket handles

SetFileCompletionNotificationModes can be unconditionally called on
non-socket handles.

The Windows poll.FD implementation still doesn't support non-socket
pollable handles yet, so this CL doesn't change any behavior.
Support for pollable non-socket handles will come in subsequent CLs.

For #19098.

Change-Id: I811a61497cfbb26acb566c20367d212335b9d551
Reviewed-on: https://go-review.googlesource.com/c/go/+/660495
Reviewed-by: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Damien Neil <[email protected]>
gopherbot pushed a commit that referenced this issue Mar 25, 2025
Setting the SIO_UDP_CONNRESET option in internal/poll.FD.Init
adds unnecessary complexity to the FD.Init signature and
implementation. Better to set it in the net package when initializing
the UDP connection, which is where conceptually it belongs.

While here, update an outdated comment in FD.Init that said the runtime
poller doesn't support I/O operations initialized by the user
outside the internal/poll package. It does support those operations
since CL 561895.

For #19098.
Updates #21172.

Change-Id: I9a70b0deafdb4619830abe2147e2d366b4c2b890
Reviewed-on: https://go-review.googlesource.com/c/go/+/660496
Auto-Submit: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
gopherbot pushed a commit that referenced this issue Mar 26, 2025
This CL adds support for async file operations on Windows. The affected
functions are Read, Write, Pread, and Pwrite.

The code has been slightly refactored to avoid duplication. Both the
async and sync variants follow the same code path, with the exception of
the async variant passes an overlapped structure to the syscalls
and supports the use of a completion port.

This doesn't change any user-facing behavior, as the os package still
sets the pollable parameter to false when calling FD.Init.

For #19098.

Change-Id: Iead6e51fa8f57e83456eb5ccdce28c2ea3846cc2
Reviewed-on: https://go-review.googlesource.com/c/go/+/660595
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/661795 mentions this issue: os,internal/poll: support I/O on overlapped handles not added to the poller

gopherbot pushed a commit that referenced this issue Mar 31, 2025
…poller

Calling syscall.ReadFile and syscall.WriteFile on overlapped handles
always need to be passed a valid *syscall.Overlapped structure, even if
the handle is not added to a IOCP (like the Go runtime poller). Else,
the syscall will fail with ERROR_INVALID_PARAMETER.

We also need to handle ERROR_IO_PENDING errors when the overlapped
handle is not added to the poller, in which case we need to block until
the operation completes.

Previous CLs already added support for overlapped handles to the poller,
mostly to keep track of the file offset independently of the file
pointer (which is not supported for overlapped handles).

Fixed #15388.
Updates #19098.

Change-Id: I2103ab892a37d0e326752ae8c2771a43c13ba42e
Reviewed-on: https://go-review.googlesource.com/c/go/+/661795
Auto-Submit: Quim Muntal <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/661955 mentions this issue: internal/poll: defer IOCP association until first IO operation

gopherbot pushed a commit that referenced this issue Apr 1, 2025
Defer the association of the IOCP to the handle until the first
I/O operation is performed.

A handle can only be associated with one IOCP at a time, so this allows
external code to associate the handle with their own IOCP and still be
able to use a FD (through os.NewFile) to pass the handle around
(e.g. to a child process standard input, output, and error) without
having to worry about the IOCP association.

This CL doesn't change any user-visible behavior, as os.NewFile still
initializes the FD as non-pollable.

For #19098.

Change-Id: Id22a49846d4fda3a66ffcc0bc1b48eb39b395dc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/661955
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/662215 mentions this issue: internal/poll: simplify execIO

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/662236 mentions this issue: os: support overlapped IO with NewFile

@qmuntal
Copy link
Member

qmuntal commented Apr 2, 2025

Cl 662236 will close this issue. It will make os.NewFile support overlapped handles by adding them to the runtime poller, which will allow using deadline on those files.

Notably, os.Open, os.Create, os.OpenFile, and os.Pipe, will remain synchronous, as changing them would be a breaking change. I plan to submit a follow-up proposal to make syscall.Open (used by os.OpenFile) map the O_NONBLOCK flag to FILE_FLAG_OVERLAPPED, which would allow users to create overlapped files without leaving the os package.

gopherbot pushed a commit that referenced this issue Apr 4, 2025
execIO has multiple return paths and multiple places where error is
mangled. This CL simplifies the function by just having one return
path.

Some more tests have been added to ensure that the error handling
is done correctly.

Updates #19098.

Change-Id: Ida0b1e85d4d123914054306e5bef8da94408b91c
Reviewed-on: https://go-review.googlesource.com/c/go/+/662215
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Auto-Submit: Carlos Amedee <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
@dmitshur dmitshur modified the milestones: Unplanned, Go1.25 Apr 4, 2025
@dmitshur dmitshur added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Apr 4, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/664455 mentions this issue: os,internal/poll: disassociate handle from IOCP in File.Fd

@qmuntal
Copy link
Member

qmuntal commented Apr 14, 2025

Reopening this issue. I think we can still do a bit better by supporting deadlines for non-overlapped handles.

@qmuntal qmuntal reopened this Apr 14, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/665315 mentions this issue: internal/poll: remove outdated tests

gopherbot pushed a commit that referenced this issue Apr 18, 2025
TestFileFdsAreInitialised and TestSerialFdsAreInitialised were added
to ensure handles passed to os.NewFile were not added to the runtime
poller. This used to be problematic because the poller could crash
if an external I/O event was received (see #21172).

This is not an issue anymore since CL 482495 and #19098.

Change-Id: I292ceae27724fefe6f438a398ebfe351dd5231d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/665315
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alex Brainman <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
gopherbot pushed a commit that referenced this issue Apr 24, 2025
Go 1.25 will gain support for overlapped IO on handles passed to
os.NewFile thanks to CL 662236. It was previously not possible to add
an overlapped handle to the Go runtime's IO completion port (IOCP),
and now happens on the first call the an IO method.

This means that there is code that relies on the fact that File.Fd
returns a handle that can always be associated with a custom IOCP.
That wouldn't be the case anymore, as a handle can only be associated
with one IOCP at a time and it must be explicitly disassociated.

To fix this breaking change, File.Fd will disassociate the handle
from the Go runtime IOCP before returning it. It is then not necessary
to defer the association until the first IO method is called, which
was recently added in CL 661955 to support this same use case, but
in a more complex and unreliable way.

Updates #19098.

Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-race,gotip-windows-amd64-longtest,gotip-windows-arm64
Change-Id: Id8a7e04d35057047c61d1733bad5bf45494b2c28
Reviewed-on: https://go-review.googlesource.com/c/go/+/664455
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Alex Brainman <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/668095 mentions this issue: os: test overlapped pipes deadlines on Windows

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/668195 mentions this issue: net,os: support converting between *os.File and net.Conn on Windows

gopherbot pushed a commit that referenced this issue Apr 30, 2025
NewFile recently added support for overlapped I/O on Windows,
which allows us to set deadlines on them, but the test coverage for
this new feature is not exhaustive.

Modify the existing pipe deadline tests to also exercise named
overlapped pipes.

Updates #19098.

Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-race,gotip-windows-amd64-longtest,gotip-windows-arm64
Change-Id: I86d284d9fb054c24959045a922cf84feeda5b5f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/668095
Reviewed-by: Alex Brainman <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
Reviewed-by: Junyang Shao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ExpertNeeded FixPending Issues that have a fix which has not yet been reviewed or submitted. OS-Windows Thinking
Projects
None yet
Development

No branches or pull requests

10 participants