Skip to content
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

Jump to bottom

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

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

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

ianlancetaylor opened this issue Feb 15, 2017 · 8 comments
Labels
ExpertNeeded OS-Windows Thinking
Milestone
Unplanned

Comments

@ianlancetaylor
Copy link
Contributor

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.
@ianlancetaylor ianlancetaylor added this to the Go1.9Maybe milestone Feb 15, 2017
@gopherbot
Copy link

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
@ianlancetaylor
os: use poller for file I/O
c05b06a 
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 <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@polarina
Copy link
Contributor

polarina commented Feb 15, 2017 •
edited

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
Contributor 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.

@bradfitz bradfitz mentioned this issue May 10, 2017
Closed
@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.

@bradfitz bradfitz modified the milestones: Go1.9Maybe, Go1.10 Jul 20, 2017
@ccbrown ccbrown mentioned this issue Sep 11, 2017
Closed
@bradfitz bradfitz modified the milestones: Go1.10, Unplanned Nov 30, 2017
@crvv crvv mentioned this issue Aug 22, 2018
Closed
@crvv crvv mentioned this issue Jan 18, 2019
Closed
@qiulaidongfeng
Copy link
Contributor

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

@qiulaidongfeng
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ExpertNeeded OS-Windows Thinking
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
8 participants
@bradfitz @polarina @rasky @ianlancetaylor @crvv @gopherbot @jstarks @qiulaidongfeng