Skip to content

Commit 75bf2a8

Browse files
committed
internal/poll: defer IOCP association until first IO operation
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]>
1 parent 7177f24 commit 75bf2a8

File tree

7 files changed

+134
-50
lines changed

7 files changed

+134
-50
lines changed

src/internal/poll/fd_plan9.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ type FD struct {
3636
isFile bool
3737
}
3838

39+
func (fd *FD) initIO() error {
40+
return nil
41+
}
42+
3943
// We need this to close out a file descriptor when it is unlocked,
4044
// but the real implementation has to live in the net package because
4145
// it uses os.File's.

src/internal/poll/fd_poll_runtime.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ func setDeadlineImpl(fd *FD, t time.Time, mode int) error {
155155
return err
156156
}
157157
defer fd.decref()
158+
159+
fd.initIO()
158160
if fd.pd.runtimeCtx == 0 {
159161
return ErrNoDeadline
160162
}

src/internal/poll/fd_unix.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ type FD struct {
4747
isFile bool
4848
}
4949

50+
func (fd *FD) initIO() error {
51+
return nil
52+
}
53+
5054
// Init initializes the FD. The Sysfd field should already be set.
5155
// This can be called multiple times on a single FD.
5256
// The net argument is a network name from the net package (e.g., "tcp"),

src/internal/poll/fd_windows.go

Lines changed: 73 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ func (o *operation) InitMsg(p []byte, oob []byte) {
157157
// It supports both synchronous and asynchronous IO.
158158
func execIO(o *operation, submit func(o *operation) error) (int, error) {
159159
fd := o.fd
160+
fd.initIO()
160161
// Notify runtime netpoll about starting IO.
161162
err := fd.pd.prepare(int(o.mode), fd.isFile)
162163
if err != nil {
@@ -297,8 +298,13 @@ type FD struct {
297298
// The kind of this file.
298299
kind fileKind
299300

300-
// Whether FILE_FLAG_OVERLAPPED was not set when opening the file
301+
// Whether FILE_FLAG_OVERLAPPED was not set when opening the file.
301302
isBlocking bool
303+
304+
// Initialization parameters.
305+
initIOOnce sync.Once
306+
initIOErr error // only used in the net package
307+
initPollable bool // value passed to [FD.Init]
302308
}
303309

304310
// setOffset sets the offset fields of the overlapped object
@@ -336,7 +342,51 @@ const (
336342
)
337343

338344
// logInitFD is set by tests to enable file descriptor initialization logging.
339-
var logInitFD func(net string, fd *FD, err error)
345+
var logInitFD func(net int, fd *FD, err error)
346+
347+
func (fd *FD) initIO() error {
348+
fd.initIOOnce.Do(func() {
349+
if fd.initPollable {
350+
// The runtime poller will ignore I/O completion
351+
// notifications not initiated by this package,
352+
// so it is safe to add handles owned by the caller.
353+
//
354+
// If we could not add the handle to the runtime poller,
355+
// assume the handle hasn't been opened for overlapped I/O.
356+
fd.initIOErr = fd.pd.init(fd)
357+
if fd.initIOErr != nil {
358+
fd.initPollable = false
359+
}
360+
}
361+
if logInitFD != nil {
362+
logInitFD(int(fd.kind), fd, fd.initIOErr)
363+
}
364+
if !fd.initPollable {
365+
// Handle opened for overlapped I/O (aka non-blocking) that are not added
366+
// to the runtime poller need special handling when reading and writing.
367+
var info windows.FILE_MODE_INFORMATION
368+
if err := windows.NtQueryInformationFile(fd.Sysfd, &windows.IO_STATUS_BLOCK{}, uintptr(unsafe.Pointer(&info)), uint32(unsafe.Sizeof(info)), windows.FileModeInformation); err == nil {
369+
fd.isBlocking = info.Mode&(windows.FILE_SYNCHRONOUS_IO_ALERT|windows.FILE_SYNCHRONOUS_IO_NONALERT) != 0
370+
} else {
371+
// If we fail to get the file mode information, assume the file is blocking.
372+
fd.isBlocking = true
373+
}
374+
} else {
375+
fd.rop.runtimeCtx = fd.pd.runtimeCtx
376+
fd.wop.runtimeCtx = fd.pd.runtimeCtx
377+
if fd.kind != kindNet || socketCanUseSetFileCompletionNotificationModes {
378+
// Non-socket handles can use SetFileCompletionNotificationModes without problems.
379+
err := syscall.SetFileCompletionNotificationModes(fd.Sysfd,
380+
syscall.FILE_SKIP_SET_EVENT_ON_HANDLE|syscall.FILE_SKIP_COMPLETION_PORT_ON_SUCCESS,
381+
)
382+
if err == nil {
383+
fd.skipSyncNotif = true
384+
}
385+
}
386+
}
387+
})
388+
return fd.initIOErr
389+
}
340390

341391
// Init initializes the FD. The Sysfd field should already be set.
342392
// This can be called multiple times on a single FD.
@@ -349,63 +399,42 @@ func (fd *FD) Init(net string, pollable bool) error {
349399
}
350400

351401
switch net {
352-
case "file", "dir":
402+
case "file":
353403
fd.kind = kindFile
354404
case "console":
355405
fd.kind = kindConsole
356406
case "pipe":
357407
fd.kind = kindPipe
358-
case "tcp", "tcp4", "tcp6",
359-
"udp", "udp4", "udp6",
360-
"ip", "ip4", "ip6",
361-
"unix", "unixgram", "unixpacket":
362-
fd.kind = kindNet
363408
default:
364-
return errors.New("internal error: unknown network type " + net)
409+
// We don't actually care about the various network types.
410+
fd.kind = kindNet
365411
}
366412
fd.isFile = fd.kind != kindNet
413+
fd.initPollable = pollable
367414
fd.rop.mode = 'r'
368415
fd.wop.mode = 'w'
369416
fd.rop.fd = fd
370417
fd.wop.fd = fd
371418

372-
var err error
373-
if pollable {
374-
// Note that the runtime poller will ignore I/O completion
375-
// notifications not initiated by this package,
376-
// so it is safe to add handles owned by the caller.
377-
//
378-
// If we could not add the handle to the runtime poller,
379-
// assume the handle hasn't been opened for overlapped I/O.
380-
err = fd.pd.init(fd)
381-
pollable = err == nil
382-
}
383-
if logInitFD != nil {
384-
logInitFD(net, fd, err)
385-
}
386-
if !pollable {
387-
// Handle opened for overlapped I/O (aka non-blocking) that are not added
388-
// to the runtime poller need special handling when reading and writing.
389-
var info windows.FILE_MODE_INFORMATION
390-
if err := windows.NtQueryInformationFile(fd.Sysfd, &windows.IO_STATUS_BLOCK{}, uintptr(unsafe.Pointer(&info)), uint32(unsafe.Sizeof(info)), windows.FileModeInformation); err == nil {
391-
fd.isBlocking = info.Mode&(windows.FILE_SYNCHRONOUS_IO_ALERT|windows.FILE_SYNCHRONOUS_IO_NONALERT) != 0
392-
} else {
393-
// If we fail to get the file mode information, assume the file is blocking.
394-
fd.isBlocking = true
395-
}
396-
return err
397-
}
398-
if fd.kind != kindNet || socketCanUseSetFileCompletionNotificationModes {
399-
// Non-socket handles can use SetFileCompletionNotificationModes without problems.
400-
err := syscall.SetFileCompletionNotificationModes(fd.Sysfd,
401-
syscall.FILE_SKIP_SET_EVENT_ON_HANDLE|syscall.FILE_SKIP_COMPLETION_PORT_ON_SUCCESS,
402-
)
403-
if err == nil {
404-
fd.skipSyncNotif = true
419+
// A file handle (and its duplicated handles) can only be associated
420+
// with one IOCP. A new association will fail if the handle is already
421+
// associated. Defer the association until the first I/O operation so that
422+
// overlapped handles passed in os.NewFile have a chance to be used
423+
// with an external IOCP. This is common case, for example, when calling
424+
// os.NewFile on a handle just to pass it to a exec.Command standard
425+
// input/output/error. If the association fails, the I/O operations
426+
// will be performed synchronously.
427+
if fd.kind == kindNet {
428+
// The net package is the only consumer that requires overlapped
429+
// handles and that cares about handle IOCP association errors.
430+
// We can should do the IOCP association here.
431+
return fd.initIO()
432+
} else {
433+
if logInitFD != nil {
434+
// For testing.
435+
logInitFD(int(fd.kind), fd, nil)
405436
}
406437
}
407-
fd.rop.runtimeCtx = fd.pd.runtimeCtx
408-
fd.wop.runtimeCtx = fd.pd.runtimeCtx
409438
return nil
410439
}
411440

src/internal/poll/fd_windows_test.go

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
)
2424

2525
type loggedFD struct {
26-
Net string
26+
Net int
2727
FD *poll.FD
2828
Err error
2929
}
@@ -33,7 +33,7 @@ var (
3333
loggedFDs map[syscall.Handle]*loggedFD
3434
)
3535

36-
func logFD(net string, fd *poll.FD, err error) {
36+
func logFD(net int, fd *poll.FD, err error) {
3737
logMu.Lock()
3838
defer logMu.Unlock()
3939

@@ -201,10 +201,6 @@ func newFD(t testing.TB, h syscall.Handle, kind string, overlapped, pollable boo
201201
if overlapped && err != nil {
202202
// Overlapped file handles should not error.
203203
t.Fatal(err)
204-
} else if !overlapped && pollable && err == nil {
205-
// Non-overlapped file handles should return an error but still
206-
// be usable as sync handles.
207-
t.Fatal("expected error for non-overlapped file handle")
208204
}
209205
return &fd
210206
}
@@ -454,6 +450,24 @@ func TestPipeWriteEOF(t *testing.T) {
454450
}
455451
}
456452

453+
func TestPipeReadTimeout(t *testing.T) {
454+
t.Parallel()
455+
name := pipeName()
456+
_ = newBytePipe(t, name, true, true)
457+
file := newFile(t, name, true, true)
458+
459+
err := file.SetReadDeadline(time.Now().Add(time.Millisecond))
460+
if err != nil {
461+
t.Fatal(err)
462+
}
463+
464+
var buf [10]byte
465+
_, err = file.Read(buf[:])
466+
if err != poll.ErrDeadlineExceeded {
467+
t.Errorf("expected deadline exceeded, got %v", err)
468+
}
469+
}
470+
457471
func TestPipeCanceled(t *testing.T) {
458472
t.Parallel()
459473
name := pipeName()
@@ -488,6 +502,26 @@ func TestPipeCanceled(t *testing.T) {
488502
}
489503
}
490504

505+
func TestPipeExternalIOCP(t *testing.T) {
506+
// Test that a caller can associate an overlapped handle to an external IOCP
507+
// even when the handle is also associated to a poll.FD. Also test that
508+
// the FD can still perform I/O after the association.
509+
t.Parallel()
510+
name := pipeName()
511+
pipe := newMessagePipe(t, name, true, true)
512+
_ = newFile(t, name, true, true) // Just open a pipe client
513+
514+
_, err := windows.CreateIoCompletionPort(syscall.Handle(pipe.Sysfd), 0, 0, 1)
515+
if err != nil {
516+
t.Fatal(err)
517+
}
518+
519+
_, err = pipe.Write([]byte("hello"))
520+
if err != nil {
521+
t.Fatal(err)
522+
}
523+
}
524+
491525
func BenchmarkReadOverlapped(b *testing.B) {
492526
benchmarkRead(b, true)
493527
}

src/internal/syscall/windows/syscall_windows.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,7 @@ const (
513513
PIPE_TYPE_MESSAGE = 0x00000004
514514
)
515515

516+
//sys CreateIoCompletionPort(filehandle syscall.Handle, cphandle syscall.Handle, key uintptr, threadcnt uint32) (handle syscall.Handle, err error)
516517
//sys GetOverlappedResult(handle syscall.Handle, overlapped *syscall.Overlapped, done *uint32, wait bool) (err error)
517518
//sys CreateNamedPipe(name *uint16, flags uint32, pipeMode uint32, maxInstances uint32, outSize uint32, inSize uint32, defaultTimeout uint32, sa *syscall.SecurityAttributes) (handle syscall.Handle, err error) [failretval==syscall.InvalidHandle] = CreateNamedPipeW
518519

src/internal/syscall/windows/zsyscall_windows.go

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)