internal/poll: don't return from Close until descriptor is closed

This permits the program to reliably know that when the Close method
returns, the descriptor has definitely been closed. This matters at
least for listeners.

Fixes #21856
Updates #7970

Change-Id: I1fd0cfd2333649e6e67c6ae956e19fdff3a35a83
Reviewed-on: https://go-review.googlesource.com/66150
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <joetsai@google.com>
This commit is contained in:
Ian Lance Taylor 2017-09-25 20:49:37 -07:00
parent 8e11cb3f3b
commit 382d4928b8
3 changed files with 50 additions and 2 deletions

View file

@ -26,6 +26,9 @@ type FD struct {
// Writev cache. // Writev cache.
iovecs *[]syscall.Iovec iovecs *[]syscall.Iovec
// Semaphore signaled when file is closed.
csema uint32
// Whether this is a streaming descriptor, as opposed to a // Whether this is a streaming descriptor, as opposed to a
// packet-based descriptor like a UDP socket. Immutable. // packet-based descriptor like a UDP socket. Immutable.
IsStream bool IsStream bool
@ -62,6 +65,7 @@ func (fd *FD) destroy() error {
fd.pd.close() fd.pd.close()
err := CloseFunc(fd.Sysfd) err := CloseFunc(fd.Sysfd)
fd.Sysfd = -1 fd.Sysfd = -1
runtime_Semrelease(&fd.csema)
return err return err
} }
@ -79,7 +83,11 @@ func (fd *FD) Close() error {
fd.pd.evict() fd.pd.evict()
// The call to decref will call destroy if there are no other // The call to decref will call destroy if there are no other
// references. // references.
return fd.decref() err := fd.decref()
// Wait until the descriptor is closed. If this was the only
// reference, it is already closed.
runtime_Semacquire(&fd.csema)
return err
} }
// Shutdown wraps the shutdown network call. // Shutdown wraps the shutdown network call.

View file

@ -278,6 +278,9 @@ type FD struct {
readbyte []byte // buffer to hold decoding of readuint16 from utf16 to utf8 readbyte []byte // buffer to hold decoding of readuint16 from utf16 to utf8
readbyteOffset int // readbyte[readOffset:] is yet to be consumed with file.Read readbyteOffset int // readbyte[readOffset:] is yet to be consumed with file.Read
// Semaphore signaled when file is closed.
csema uint32
skipSyncNotif bool skipSyncNotif bool
// Whether this is a streaming descriptor, as opposed to a // Whether this is a streaming descriptor, as opposed to a
@ -399,6 +402,7 @@ func (fd *FD) destroy() error {
err = CloseFunc(fd.Sysfd) err = CloseFunc(fd.Sysfd)
} }
fd.Sysfd = syscall.InvalidHandle fd.Sysfd = syscall.InvalidHandle
runtime_Semrelease(&fd.csema)
return err return err
} }
@ -410,7 +414,11 @@ func (fd *FD) Close() error {
} }
// unblock pending reader and writer // unblock pending reader and writer
fd.pd.evict() fd.pd.evict()
return fd.decref() err := fd.decref()
// Wait until the descriptor is closed. If this was the only
// reference, it is already closed.
runtime_Semacquire(&fd.csema)
return err
} }
// Shutdown wraps the shutdown network call. // Shutdown wraps the shutdown network call.

View file

@ -13,6 +13,7 @@ import (
"runtime" "runtime"
"syscall" "syscall"
"testing" "testing"
"time"
) )
func (ln *TCPListener) port() string { func (ln *TCPListener) port() string {
@ -696,3 +697,34 @@ func multicastRIBContains(ip IP) (bool, error) {
} }
return false, nil return false, nil
} }
// Issue 21856.
func TestClosingListener(t *testing.T) {
listener, err := Listen("tcp", ":0")
if err != nil {
t.Fatal(err)
}
addr := listener.Addr()
go func() {
for {
c, err := listener.Accept()
if err != nil {
return
}
c.Close()
}
}()
// Let the goroutine start. We don't sleep long: if the
// goroutine doesn't start, the test will pass without really
// testing anything, which is OK.
time.Sleep(time.Millisecond)
listener.Close()
_, err = Listen("tcp", addr.String())
if err != nil {
t.Error(err)
}
}