ソースを参照

Fixes for go test -race -count 3

The race conditions were caused by old Reader go routines
from other tests!

Putting the channels in the Door struct seemed to "resolve"
those issues.

I still get random failures in test_menu, menu.go 104
index out of range -3 which would be -2 disconnect from the
GetKey() call.
Steve Thielemann 3 年 前
コミット
194bf7f898

+ 14 - 10
door/door.go

@@ -27,6 +27,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 )
 
@@ -75,14 +76,17 @@ type DropfileConfig struct {
 }
 
 type Door struct {
-	Config       DropfileConfig
-	READFD       int
-	WRITEFD      int
-	Disconnected bool
-	TimeOut      time.Time // Fixed point in time, when time expires
-	StartTime    time.Time
-	Pushback     FIFOBuffer
-	LastColor    []int
+	Config        DropfileConfig
+	READFD        int
+	WRITEFD       int
+	Disconnected  bool
+	TimeOut       time.Time // Fixed point in time, when time expires
+	StartTime     time.Time
+	Pushback      FIFOBuffer
+	LastColor     []int
+	readerChannel chan byte
+	writerChannel chan string
+	writerMutex   sync.Mutex
 }
 
 // Return the amount of time left as time.Duration
@@ -271,8 +275,8 @@ func (d *Door) Init(doorname string) {
 
 	log.Printf("BBS %s, User %s / Handle %s / File %d\n", d.Config.BBSID, d.Config.Real_name, d.Config.Handle, d.Config.Comm_handle)
 
-	readerChannel = make(chan byte)
-	writerChannel = make(chan string)
+	d.readerChannel = make(chan byte)
+	d.writerChannel = make(chan string)
 
 	d.setupChannels()
 

+ 2 - 2
door/door_linux.go

@@ -1,6 +1,6 @@
 package door
 
 func (d *Door) setupChannels() {
-	go Reader(d.Config.Comm_handle)
-	go Writer(d.Config.Comm_handle)
+	go Reader(d.Config.Comm_handle, &d.readerChannel)
+	go Writer(d.Config.Comm_handle, &d.writerChannel)
 }

+ 4 - 0
door/door_test.go

@@ -202,4 +202,8 @@ func TestDetectFail(t *testing.T) {
 		t.Errorf("Expected 0, 0, got Width %d, Height %d", Width, Height)
 	}
 
+	server.Close()
+	client.Close()
+
+	// time.Sleep(time.Duration(1) * time.Second)
 }

+ 2 - 2
door/door_windows.go

@@ -3,6 +3,6 @@ package door
 import "syscall"
 
 func (d *Door) setupChannels() {
-	go Reader(syscall.Handle(d.Config.Comm_handle))
-	go Writer(syscall.Handle(d.Config.Comm_handle))
+	go Reader(syscall.Handle(d.Config.Comm_handle), &d.readerChannel)
+	go Writer(syscall.Handle(d.Config.Comm_handle), &d.writerChannel)
 }

+ 2 - 4
door/input.go

@@ -34,15 +34,13 @@ const (
 	XKEY_UNKNOWN     = 0x1111
 )
 
-var readerChannel chan byte
-
 // Low level read key function.
 // This gets the raw keys from the client, it doesn't handle extended keys,
 // functions, arrows.
 // Return key, or -1 (Timeout/No key available), -2 hangup
 func (d *Door) getch() int {
 	select {
-	case res, ok := <-readerChannel:
+	case res, ok := <-d.readerChannel:
 		if ok {
 			return int(res)
 		} else {
@@ -272,7 +270,7 @@ func (d *Door) WaitKey(secs int64, usecs int64) int {
 	timeout := time.Duration(secs)*time.Second + time.Duration(usecs)*time.Microsecond
 
 	select {
-	case res, ok := <-readerChannel:
+	case res, ok := <-d.readerChannel:
 		if ok {
 			d.Pushback.Push(int(res))
 			return d.GetKey()

+ 9 - 39
door/input_linux.go

@@ -5,59 +5,29 @@ import (
 	"syscall"
 )
 
-func Reader(handle int) {
+func Reader(handle int, readerChannel *chan byte) {
 	// I don't need the select anymore.  Let the read block.
+	defer func() {
+		if err := recover(); err != nil {
+			log.Printf("Reader: %#v\n", err)
+		}
+	}()
 
 	buffer := make([]byte, 1)
 	for {
 		read, err := syscall.Read(handle, buffer)
 		if err != nil {
 			log.Printf("Reader ERR: %#v\n", err)
-			close(readerChannel)
+			close(*readerChannel)
 			break
 		}
 		if read == 1 {
-			readerChannel <- buffer[0]
+			*readerChannel <- buffer[0]
 		} else {
 			log.Printf("READ FAILED %d\n", read)
-			close(readerChannel)
+			close(*readerChannel)
 			break
 		}
 	}
 }
 
-/*
-// Low level read key function.
-// This gets the raw keys from the client, it doesn't handle extended keys,
-// functions, arrows.
-// Return key, or -1 (Timeout/No key available), -2 hangup
-func (d *Door) getch() int {
-	var fdsetRead syscall.FdSet
-	clearAll(&fdsetRead)
-	set(&fdsetRead, d.READFD)
-
-	// 100 Usec seems like a good value, works with QModem in dosbox.
-	timeout := syscall.Timeval{Sec: 0, Usec: 100}
-	v, err := syscall.Select(d.READFD+1, &fdsetRead, nil, nil, &timeout)
-	if v == -1 {
-		// hangup
-		return -2
-	}
-	if v == 0 {
-		// timeout
-		return -1
-	}
-
-	buffer := make([]byte, 1)
-	r, err := syscall.Read(d.READFD, buffer)
-	if r != 1 {
-		// I'm getting write error here... (when disconnected)
-		log.Printf("Read said ready, but didn't read a character %d %v.\n", r, err)
-		// hangup
-		d.Disconnected = true
-		return -2
-	}
-	return int(buffer[0])
-}
-
-*/

+ 24 - 2
door/input_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
@@ -27,6 +28,7 @@ func TestDoorInputConnection(t *testing.T) {
 	// unicode 190x43 response
 	buffer := []byte("\x1b[1;1R\x1b[2;3R\x1b[43;190R")
 	server.Write(buffer)
+	runtime.Gosched()
 
 	// Access Fd (File descriptor) of client for dropfile
 	var fd int = socket_to_fd(client)
@@ -41,6 +43,15 @@ func TestDoorInputConnection(t *testing.T) {
 
 	d := Door{}
 
+	// Because we're not the only one calling door.Init(), the
+	// door global variables might be from a previous test run.
+
+	Unicode = false
+	CP437 = false
+	Full_CP437 = false
+	Width = 0
+	Height = 0
+
 	// If I call d.Init() more then once flag complains about flag redefined.
 	// Reset flags
 	flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
@@ -98,10 +109,15 @@ func TestDoorInputConnection(t *testing.T) {
 	for send, get := range keytest {
 		buffer := []byte(send)
 		server.Write(buffer)
+		runtime.Gosched()
 
 		recv := make([]int, 0)
 		for {
-			input := d.WaitKey(0, 50)
+			// input := d.WaitKey(0, 50)
+			// running go test -count > 1 sometimes fails --
+			// not getting all of the data we Write above.
+			// data shows up on next read.
+			input := d.WaitKey(0, 100)
 			if input != -1 {
 				recv = append(recv, input)
 			} else {
@@ -147,6 +163,8 @@ func TestDoorInputConnection(t *testing.T) {
 	// Input test
 	buffer = []byte("1234567890\r")
 	server.Write(buffer)
+	runtime.Gosched()
+
 	input := d.Input(5)
 
 	if input != "12345" {
@@ -169,6 +187,8 @@ func TestDoorInputConnection(t *testing.T) {
 
 	buffer = []byte("12345678\x08\x089\r")
 	server.Write(buffer)
+	runtime.Gosched()
+
 	input = d.Input(5)
 
 	if input != "1239" {
@@ -177,6 +197,8 @@ func TestDoorInputConnection(t *testing.T) {
 
 	buffer = []byte("12\x08\x08\x08987\x00\x48654321\r")
 	server.Write(buffer)
+	runtime.Gosched()
+
 	input = d.Input(5)
 
 	if input != "98765" {
@@ -206,6 +228,7 @@ func TestDoorInputConnection(t *testing.T) {
 		t.Errorf("Expected -2 (hangup), got %d", hungup)
 	}
 	client.Close()
+	runtime.Gosched()
 
 	blank := d.Input(5)
 
@@ -218,7 +241,6 @@ func TestDoorInputConnection(t *testing.T) {
 	if hungup != -2 {
 		t.Errorf("Expected -2 (hangup), got %d", hungup)
 	}
-
 }
 
 func TestDisplayInput(t *testing.T) {

+ 10 - 4
door/input_windows.go

@@ -5,7 +5,13 @@ import (
 	"syscall"
 )
 
-func Reader(handle syscall.Handle) {
+func Reader(handle syscall.Handle, readerChannel *chan byte) {
+	defer func() {
+		if err := recover(); err != nil {
+			log.Printf("Reader: %#v\n", err)
+		}
+	}()
+
 	buffer := make([]byte, 1)
 	WSA_Buffer := syscall.WSABuf{Len: 1, Buf: &buffer[0]}
 	read := uint32(0)
@@ -15,15 +21,15 @@ func Reader(handle syscall.Handle) {
 		err := syscall.WSARecv(handle, &WSA_Buffer, 1, &read, &flags, nil, nil)
 		if err != nil {
 			log.Printf("Reader ERR: %#v\n", err)
-			close(readerChannel)
+			close(*readerChannel)
 			break
 		}
 
 		if read == 1 {
-			readerChannel <- buffer[0]
+			*readerChannel <- buffer[0]
 		} else {
 			log.Printf("READ FAILED %d\n", read)
-			close(readerChannel)
+			close(*readerChannel)
 			break
 		}
 	}

+ 24 - 1
door/menu_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
@@ -64,6 +65,7 @@ func TestMenuConnection(t *testing.T) {
 	// unicode 90x40 response
 	buffer := []byte("\x1b[1;1R\x1b[2;3R\x1b[40;90R")
 	server.Write(buffer)
+	runtime.Gosched()
 
 	// Access Fd (File descriptor) of client for dropfile
 	var fd int = socket_to_fd(client)
@@ -135,8 +137,13 @@ func TestMenuConnection(t *testing.T) {
 	// BUG:  I can't use \x1b[A because of the way extended codes work there.
 	keys := "\x00\x50\x00\x50\r" // down, down, ENTER
 	server.Write([]byte(keys))
+	runtime.Gosched()
 
 	choice = m.Choose(&d)
+	if choice < 0 {
+		t.Errorf("Error <0 from Choose: %d", choice)
+	}
+
 	option := m.GetOption(choice)
 
 	if choice != 3 {
@@ -164,8 +171,14 @@ func TestMenuConnection(t *testing.T) {
 
 	keys = "\x00\x48\r" // up, ENTER
 	server.Write([]byte(keys))
+	runtime.Gosched()
+
 	m.Chosen = 1
 	choice = m.Choose(&d)
+	if choice < 0 {
+		t.Errorf("Error <0 from Choose: %d", choice)
+	}
+
 	option = m.GetOption(choice)
 
 	if choice != 1 {
@@ -190,6 +203,8 @@ func TestMenuConnection(t *testing.T) {
 
 	keys = "\x00\x4f\r" // END, ENTER
 	server.Write([]byte(keys))
+	runtime.Gosched()
+
 	m.Chosen = 1
 	choice = m.Choose(&d)
 	option = m.GetOption(choice)
@@ -209,6 +224,8 @@ func TestMenuConnection(t *testing.T) {
 
 	keys = "\x00\x47\r" // HOME, ENTER
 	server.Write([]byte(keys))
+	runtime.Gosched()
+
 	m.Chosen = 2
 	choice = m.Choose(&d)
 	option = m.GetOption(choice)
@@ -227,6 +244,8 @@ func TestMenuConnection(t *testing.T) {
 	// output = string(buffer[:r])
 
 	server.Write([]byte("B"))
+	runtime.Gosched()
+
 	m.Chosen = 0
 	choice = m.Choose(&d)
 	option = m.GetOption(choice)
@@ -236,7 +255,9 @@ func TestMenuConnection(t *testing.T) {
 	}
 
 	// Read the display output
-	server.SetReadDeadline(time.Now().Add(time.Millisecond * 50))
+
+	// i/o timeout
+	server.SetReadDeadline(time.Now().Add(time.Millisecond * 100))
 	r, err = server.Read(buffer)
 	if err != nil {
 		t.Errorf("server.Read: %s", err)
@@ -246,6 +267,8 @@ func TestMenuConnection(t *testing.T) {
 
 	keys = "2\r" // "Down", ENTER
 	server.Write([]byte(keys))
+	runtime.Gosched()
+
 	m.Chosen = 1
 	choice = m.Choose(&d)
 	option = m.GetOption(choice)

+ 8 - 2
door/write.go

@@ -128,7 +128,7 @@ ok == false
 writing to a closed channel is a panic.
 */
 
-var writerChannel chan string
+// var writeMutex sync.Mutex
 
 // Write string to client.
 func (d *Door) Write(output string) {
@@ -136,6 +136,12 @@ func (d *Door) Write(output string) {
 		return
 	}
 
+	// There is a potential race condition here --
+	// We could be updating d.LastColor when called again.
+
+	d.writerMutex.Lock()
+	defer d.writerMutex.Unlock()
+
 	defer func() {
 		if r := recover(); r != nil {
 			log.Println("Write error/HANGUP.")
@@ -159,5 +165,5 @@ func (d *Door) Write(output string) {
 		log.Printf("Parse: [%s]\n", temp)
 	*/
 
-	writerChannel <- output
+	d.writerChannel <- output
 }

+ 3 - 3
door/write_linux.go

@@ -4,12 +4,12 @@ import (
 	"syscall"
 )
 
-func Writer(handle int) {
-	for output := range writerChannel {
+func Writer(handle int, writerChannel *chan string) {
+	for output := range *writerChannel {
 		buffer := []byte(output)
 		n, err := syscall.Write(handle, buffer)
 		if (err != nil) || (n != len(buffer)) {
-			close(writerChannel)
+			close(*writerChannel)
 			break
 		}
 	}

+ 3 - 3
door/write_windows.go

@@ -5,8 +5,8 @@ import (
 	"syscall"
 )
 
-func Writer(handle syscall.Handle) {
-	for output := range writerChannel {
+func Writer(handle syscall.Handle, writerChannel *chan string) {
+	for output := range *writerChannel {
 		l := uint32(len(output))
 		buffer := []byte(output)
 		WSA_Buffer := syscall.WSABuf{Len: uint32(l), Buf: &buffer[0]}
@@ -18,7 +18,7 @@ func Writer(handle syscall.Handle) {
 		}
 
 		if (err != nil) || (l != DataWrite) {
-			close(writerChannel)
+			close(*writerChannel)
 			break
 		}
 	}