From f8c1f56fd31f0f8f066ae4b7677f475336875f0c Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Sun, 6 Nov 2016 18:30:40 +0100 Subject: [PATCH 1/7] Added unix-pipe wrapper --- pipe_unix.go | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 pipe_unix.go diff --git a/pipe_unix.go b/pipe_unix.go new file mode 100644 index 0000000..066f258 --- /dev/null +++ b/pipe_unix.go @@ -0,0 +1,53 @@ +// +// 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 serial // import "go.bug.st/serial.v1" + +import "syscall" + +// pipe is a small wrapper around unix-pipe syscall functions +type pipe struct { + rd int + wr int +} + +func newPipe() (*pipe, error) { + fds := []int{0, 0} + if err := syscall.Pipe(fds); err != nil { + return nil, err + } + return &pipe{rd: fds[0], wr: fds[1]}, nil +} + +func (p *pipe) ReadFD() int { + return p.rd +} + +func (p *pipe) WriteFD() int { + return p.wr +} + +func (p *pipe) Write(data []byte) (int, error) { + return syscall.Write(p.wr, data) +} + +func (p *pipe) Read(data []byte) (int, error) { + return syscall.Read(p.rd, data) +} + +func (p *pipe) Close() error { + err1 := syscall.Close(p.rd) + err2 := syscall.Close(p.wr) + if err1 != nil { + return err1 + } + if err2 != nil { + return err2 + } + return nil +} From 12cc48dd24f43c4d721faae88768247aa291c86b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Sun, 6 Nov 2016 18:30:49 +0100 Subject: [PATCH 2/7] unix: avoid Read blocking forever if called before Close --- serial_unix.go | 61 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 6 deletions(-) diff --git a/serial_unix.go b/serial_unix.go index 2368cdc..bbbcbce 100644 --- a/serial_unix.go +++ b/serial_unix.go @@ -8,22 +8,63 @@ package serial // import "go.bug.st/serial.v1" -import "io/ioutil" -import "regexp" -import "strings" -import "syscall" -import "unsafe" +import ( + "io/ioutil" + "regexp" + "strings" + "sync" + "syscall" + "unsafe" + + "github.com/creack/goselect" +) type unixPort struct { handle int + + closeLock sync.RWMutex + closeSignal *pipe } func (port *unixPort) Close() error { + // Close port port.releaseExclusiveAccess() - return syscall.Close(port.handle) + if err := syscall.Close(port.handle); err != nil { + return err + } + + // 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() + + r := &goselect.FDSet{} + r.Set(uintptr(port.handle)) + r.Set(uintptr(port.closeSignal.ReadFD())) + e := &goselect.FDSet{} + e.Set(uintptr(port.handle)) + e.Set(uintptr(port.closeSignal.ReadFD())) + + max := port.closeSignal.ReadFD() + if port.handle > max { + max = port.handle + } + if err = goselect.Select(max+1, r, nil, e, -1); err != nil { + return 0, err + } return syscall.Read(port.handle, p) } @@ -93,6 +134,14 @@ func nativeOpen(portName string, mode *Mode) (*unixPort, error) { port.acquireExclusiveAccess() + // This pipe is used as a signal to cancel blocking Read or Write + pipe, err := newPipe() + if err != nil { + port.Close() + return nil, &PortError{code: InvalidSerialPort, causedBy: err} + } + port.closeSignal = pipe + return port, nil } From eee219b43f92eb113ddceeaf6f411c17e08fe909 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 8 Nov 2016 12:31:18 +0100 Subject: [PATCH 3/7] posix: added check for port closing --- serial_unix.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/serial_unix.go b/serial_unix.go index bbbcbce..84488b5 100644 --- a/serial_unix.go +++ b/serial_unix.go @@ -33,16 +33,18 @@ func (port *unixPort) Close() error { return err } - // Send close signal to all pending reads (if any) - port.closeSignal.Write([]byte{0}) + 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() + // 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 + // Close signaling pipe + if err := port.closeSignal.Close(); err != nil { + return err + } } return nil } From 152558831e919803fd60169bceaa301518f47627 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Sun, 13 Nov 2016 23:44:13 +0100 Subject: [PATCH 4/7] Moved Pipe{} into own sub-package It may be moved into a separate project at some point, who knows... --- pipe_unix.go | 53 ------------------------------- serial_unix.go | 8 +++-- unixutils/pipe.go | 80 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 56 deletions(-) delete mode 100644 pipe_unix.go create mode 100644 unixutils/pipe.go diff --git a/pipe_unix.go b/pipe_unix.go deleted file mode 100644 index 066f258..0000000 --- a/pipe_unix.go +++ /dev/null @@ -1,53 +0,0 @@ -// -// 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 serial // import "go.bug.st/serial.v1" - -import "syscall" - -// pipe is a small wrapper around unix-pipe syscall functions -type pipe struct { - rd int - wr int -} - -func newPipe() (*pipe, error) { - fds := []int{0, 0} - if err := syscall.Pipe(fds); err != nil { - return nil, err - } - return &pipe{rd: fds[0], wr: fds[1]}, nil -} - -func (p *pipe) ReadFD() int { - return p.rd -} - -func (p *pipe) WriteFD() int { - return p.wr -} - -func (p *pipe) Write(data []byte) (int, error) { - return syscall.Write(p.wr, data) -} - -func (p *pipe) Read(data []byte) (int, error) { - return syscall.Read(p.rd, data) -} - -func (p *pipe) Close() error { - err1 := syscall.Close(p.rd) - err2 := syscall.Close(p.wr) - if err1 != nil { - return err1 - } - if err2 != nil { - return err2 - } - return nil -} diff --git a/serial_unix.go b/serial_unix.go index 84488b5..a4ca1ee 100644 --- a/serial_unix.go +++ b/serial_unix.go @@ -16,6 +16,8 @@ import ( "syscall" "unsafe" + "go.bug.st/serial.v1/unixutils" + "github.com/creack/goselect" ) @@ -23,7 +25,7 @@ type unixPort struct { handle int closeLock sync.RWMutex - closeSignal *pipe + closeSignal *unixutils.Pipe } func (port *unixPort) Close() error { @@ -137,8 +139,8 @@ func nativeOpen(portName string, mode *Mode) (*unixPort, error) { port.acquireExclusiveAccess() // This pipe is used as a signal to cancel blocking Read or Write - pipe, err := newPipe() - if err != nil { + pipe := &unixutils.Pipe{} + if err := pipe.Open(); err != nil { port.Close() return nil, &PortError{code: InvalidSerialPort, causedBy: err} } diff --git a/unixutils/pipe.go b/unixutils/pipe.go new file mode 100644 index 0000000..567c739 --- /dev/null +++ b/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 +} From 6eedab17b4eed27850ea899bd122d04b2e800f38 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 14 Nov 2016 00:33:28 +0100 Subject: [PATCH 5/7] Added wrapper around goselect library. This simplifies handling of select syscall. --- serial_unix.go | 17 ++------ unixutils/select.go | 101 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 unixutils/select.go diff --git a/serial_unix.go b/serial_unix.go index a4ca1ee..696b3b5 100644 --- a/serial_unix.go +++ b/serial_unix.go @@ -17,8 +17,6 @@ import ( "unsafe" "go.bug.st/serial.v1/unixutils" - - "github.com/creack/goselect" ) type unixPort struct { @@ -55,18 +53,9 @@ func (port *unixPort) Read(p []byte) (n int, err error) { port.closeLock.RLock() defer port.closeLock.RUnlock() - r := &goselect.FDSet{} - r.Set(uintptr(port.handle)) - r.Set(uintptr(port.closeSignal.ReadFD())) - e := &goselect.FDSet{} - e.Set(uintptr(port.handle)) - e.Set(uintptr(port.closeSignal.ReadFD())) - - max := port.closeSignal.ReadFD() - if port.handle > max { - max = port.handle - } - if err = goselect.Select(max+1, r, nil, e, -1); err != nil { + fds := unixutils.NewFDSet(port.handle, port.closeSignal.ReadFD()) + res, err := unixutils.Select(fds, nil, fds, -1) + if err != nil { return 0, err } return syscall.Read(port.handle, p) diff --git a/unixutils/select.go b/unixutils/select.go new file mode 100644 index 0000000..bb73b1b --- /dev/null +++ b/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 = ©OfRd + // Determine max fd. + max = rd.max + } + if wr != nil { + // fdsets are copied so the parameters are left untouched + copyOfWr := wr.set + res.writeable = ©OfWr + // 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 = ©OfEr + // 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 +} From d50de5cbafa06d8f9396fd4507b07be65a786d56 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 14 Nov 2016 00:31:01 +0100 Subject: [PATCH 6/7] unix: Fixed race condition while closing serial port. Added PortClosed error --- serial.go | 4 ++++ serial_unix.go | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/serial.go b/serial.go index 6b932d0..eb8a301 100644 --- a/serial.go +++ b/serial.go @@ -100,6 +100,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 @@ -123,6 +125,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" } diff --git a/serial_unix.go b/serial_unix.go index 696b3b5..49f5903 100644 --- a/serial_unix.go +++ b/serial_unix.go @@ -24,6 +24,7 @@ type unixPort struct { closeLock sync.RWMutex closeSignal *unixutils.Pipe + opened bool } func (port *unixPort) Close() error { @@ -32,6 +33,7 @@ func (port *unixPort) Close() error { 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) @@ -52,12 +54,18 @@ func (port *unixPort) Close() error { 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) } @@ -98,6 +106,7 @@ func nativeOpen(portName string, mode *Mode) (*unixPort, error) { } port := &unixPort{ handle: h, + opened: true, } // Setup serial port @@ -127,7 +136,7 @@ func nativeOpen(portName string, mode *Mode) (*unixPort, error) { port.acquireExclusiveAccess() - // This pipe is used as a signal to cancel blocking Read or Write + // This pipe is used as a signal to cancel blocking Read pipe := &unixutils.Pipe{} if err := pipe.Open(); err != nil { port.Close() From 018968a9e6253ae036160d28decea76cad622ec0 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 14 Nov 2016 00:51:48 +0100 Subject: [PATCH 7/7] Fixed travis build --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 461c930..2783c85 100644 --- a/.travis.yml +++ b/.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: