Merge remote-tracking branch 'cmaglie/fix-read-close-deadlock' into v1

.travis.yml

@@ -25,7 +25,7 @@ before_install:
 script:
   - GOARM=5 GO386=387 GOOS=$TEST_OS GOARCH=$TEST_ARCH go get golang.org/x/sys/windows
   - GOARM=5 GO386=387 GOOS=$TEST_OS GOARCH=$TEST_ARCH go build -v ./...
-  - GOARM=5 GO386=387 GOOS=$TEST_OS GOARCH=$TEST_ARCH go test -c -v ./...
+  - GOARM=5 GO386=387 GOOS=$TEST_OS GOARCH=$TEST_ARCH go test -c -v go.bug.st/serial.v1
 
 notifications:
   email:

serial.go

@@ -119,6 +119,8 @@ const (
 	InvalidStopBits
 	// ErrorEnumeratingPorts an error occurred while listing serial port
 	ErrorEnumeratingPorts
+	// PortClosed the port has been closed while the operation is in progress
+	PortClosed
 )
 
 // EncodedErrorString returns a string explaining the error code
@@ -142,6 +144,8 @@ func (e PortError) EncodedErrorString() string {
 		return "Port stop bits invalid or not supported"
 	case ErrorEnumeratingPorts:
 		return "Could not enumerate serial ports"
+	case PortClosed:
+		return "Port has been closed"
 	default:
 		return "Other error"
 	}

serial_unix.go

@@ -8,22 +8,64 @@
 
 package serial // import "go.bug.st/serial.v1"
 
-import "io/ioutil"
+import (
-import "regexp"
+	"io/ioutil"
-import "strings"
+	"regexp"
-import "syscall"
+	"strings"
-import "unsafe"
+	"sync"
+	"syscall"
+	"unsafe"
+
+	"go.bug.st/serial.v1/unixutils"
+)
 
 type unixPort struct {
 	handle int
+
+	closeLock   sync.RWMutex
+	closeSignal *unixutils.Pipe
+	opened      bool
 }
 
 func (port *unixPort) Close() error {
+	// Close port
 	port.releaseExclusiveAccess()
-	return syscall.Close(port.handle)
+	if err := syscall.Close(port.handle); err != nil {
+		return err
+	}
+	port.opened = false
+
+	if port.closeSignal != nil {
+		// Send close signal to all pending reads (if any)
+		port.closeSignal.Write([]byte{0})
+
+		// Wait for all readers to complete
+		port.closeLock.Lock()
+		defer port.closeLock.Unlock()
+
+		// Close signaling pipe
+		if err := port.closeSignal.Close(); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 func (port *unixPort) Read(p []byte) (n int, err error) {
+	port.closeLock.RLock()
+	defer port.closeLock.RUnlock()
+	if !port.opened {
+		return 0, &PortError{code: PortClosed}
+	}
+
+	fds := unixutils.NewFDSet(port.handle, port.closeSignal.ReadFD())
+	res, err := unixutils.Select(fds, nil, fds, -1)
+	if err != nil {
+		return 0, err
+	}
+	if res.IsReadable(port.closeSignal.ReadFD()) {
+		return 0, &PortError{code: PortClosed}
+	}
 	return syscall.Read(port.handle, p)
 }
 
@@ -103,6 +145,7 @@ func nativeOpen(portName string, mode *Mode) (*unixPort, error) {
 	}
 	port := &unixPort{
 		handle: h,
+		opened: true,
 	}
 
 	// Setup serial port
@@ -132,6 +175,14 @@ func nativeOpen(portName string, mode *Mode) (*unixPort, error) {
 
 	port.acquireExclusiveAccess()
 
+	// This pipe is used as a signal to cancel blocking Read
+	pipe := &unixutils.Pipe{}
+	if err := pipe.Open(); err != nil {
+		port.Close()
+		return nil, &PortError{code: InvalidSerialPort, causedBy: err}
+	}
+	port.closeSignal = pipe
+
 	return port, nil
 }
 

unixutils/pipe.go

@@ -0,0 +1,80 @@
+//
+// Copyright 2014-2016 Cristian Maglie. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+//
+
+// +build linux darwin freebsd
+
+package unixutils // import "go.bug.st/serial.v1/unixutils"
+
+import "syscall"
+import "fmt"
+
+// Pipe represents a unix-pipe
+type Pipe struct {
+	opened bool
+	rd     int
+	wr     int
+}
+
+// Open creates a new pipe
+func (p *Pipe) Open() error {
+	fds := []int{0, 0}
+	if err := syscall.Pipe(fds); err != nil {
+		return err
+	}
+	p.rd = fds[0]
+	p.wr = fds[1]
+	p.opened = true
+	return nil
+}
+
+// ReadFD returns the file handle for the read side of the pipe.
+func (p *Pipe) ReadFD() int {
+	if !p.opened {
+		return -1
+	}
+	return p.rd
+}
+
+// WriteFD returns the flie handle for the write side of the pipe.
+func (p *Pipe) WriteFD() int {
+	if !p.opened {
+		return -1
+	}
+	return p.wr
+}
+
+// Write to the pipe the content of data. Returns the numbre of bytes written.
+func (p *Pipe) Write(data []byte) (int, error) {
+	if !p.opened {
+		return 0, fmt.Errorf("Pipe not opened")
+	}
+	return syscall.Write(p.wr, data)
+}
+
+// Read from the pipe into the data array. Returns the number of bytes read.
+func (p *Pipe) Read(data []byte) (int, error) {
+	if !p.opened {
+		return 0, fmt.Errorf("Pipe not opened")
+	}
+	return syscall.Read(p.rd, data)
+}
+
+// Close the pipe
+func (p *Pipe) Close() error {
+	if !p.opened {
+		return fmt.Errorf("Pipe not opened")
+	}
+	err1 := syscall.Close(p.rd)
+	err2 := syscall.Close(p.wr)
+	p.opened = false
+	if err1 != nil {
+		return err1
+	}
+	if err2 != nil {
+		return err2
+	}
+	return nil
+}

unixutils/select.go

@@ -0,0 +1,101 @@
+//
+// Copyright 2014-2016 Cristian Maglie. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+//
+
+// +build linux darwin freebsd
+
+package unixutils // "go.bug.st/serial.v1/unixutils"
+
+import (
+	"time"
+
+	"github.com/creack/goselect"
+)
+
+// FDSet is a set of file descriptors suitable for a select call
+type FDSet struct {
+	set goselect.FDSet
+	max uintptr
+}
+
+// NewFDSet creates a set of file descriptors suitable for a Select call.
+func NewFDSet(fds ...int) *FDSet {
+	s := &FDSet{}
+	s.Add(fds...)
+	return s
+}
+
+// Add adds the file descriptors passed as parameter to the FDSet.
+func (s *FDSet) Add(fds ...int) {
+	for _, fd := range fds {
+		f := uintptr(fd)
+		s.set.Set(f)
+		if f > s.max {
+			s.max = f
+		}
+	}
+}
+
+// FDResultSets contains the result of a Select operation.
+type FDResultSets struct {
+	readable  *goselect.FDSet
+	writeable *goselect.FDSet
+	errors    *goselect.FDSet
+}
+
+// IsReadable test if a file descriptor is ready to be read.
+func (r *FDResultSets) IsReadable(fd int) bool {
+	return r.readable.IsSet(uintptr(fd))
+}
+
+// IsWritable test if a file descriptor is ready to be written.
+func (r *FDResultSets) IsWritable(fd int) bool {
+	return r.writeable.IsSet(uintptr(fd))
+}
+
+// IsError test if a file descriptor is in error state.
+func (r *FDResultSets) IsError(fd int) bool {
+	return r.errors.IsSet(uintptr(fd))
+}
+
+// Select performs a select system call,
+// file descriptors in the rd set are tested for read-events,
+// file descriptors in the wd set are tested for write-events and
+// file descriptors in the er set are tested for error-events.
+// The function will block until an event happens or the timeout expires.
+// The function return an FDResultSets that contains all the file descriptor
+// that have a pending read/write/error event.
+func Select(rd, wr, er *FDSet, timeout time.Duration) (*FDResultSets, error) {
+	max := uintptr(0)
+	res := &FDResultSets{}
+	if rd != nil {
+		// fdsets are copied so the parameters are left untouched
+		copyOfRd := rd.set
+		res.readable = &copyOfRd
+		// Determine max fd.
+		max = rd.max
+	}
+	if wr != nil {
+		// fdsets are copied so the parameters are left untouched
+		copyOfWr := wr.set
+		res.writeable = &copyOfWr
+		// Determine max fd.
+		if wr.max > max {
+			max = wr.max
+		}
+	}
+	if er != nil {
+		// fdsets are copied so the parameters are left untouched
+		copyOfEr := er.set
+		res.errors = &copyOfEr
+		// Determine max fd.
+		if er.max > max {
+			max = er.max
+		}
+	}
+
+	err := goselect.Select(int(max+1), res.readable, res.writeable, res.errors, timeout)
+	return res, err
+}