Latest changes, removing atomic bool.

door/door.go

@@ -30,7 +30,6 @@ import (
 	"strconv"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"time"
 )
 
@@ -92,16 +91,34 @@ type Door struct {
 	Config         DropfileConfig
 	READFD         int
 	WRITEFD        int
-	Disconnected   int32         // atomic bool      // Has User disconnected/Hung up?
-	TimeOut        time.Time     // Fixed point in time, when time expires
-	StartTime      time.Time     // Time when User started door
-	Pushback       FIFOBuffer    // Key buffer
-	LastColor      []int         // Track the last color sent for restore color
-	readerChannel  chan byte     // Reading from the User
-	ReaderCanClose bool          // We can close the reader (in tests)
-	writerChannel  chan string   // Writing to the User
-	closeChannel   chan struct{} // Closing
-	wg             sync.WaitGroup
+	Disconnected   bool        // int32         // atomic bool      // Has User disconnected/Hung up?
+	TimeOut        time.Time   // Fixed point in time, when time expires
+	StartTime      time.Time   // Time when User started door
+	Pushback       FIFOBuffer  // Key buffer
+	LastColor      []int       // Track the last color sent for restore color
+	ReaderClosed   bool        // Reader close
+	readerChannel  chan byte   // Reading from the User
+	ReaderCanClose bool        // We can close the reader (in tests)
+	WriterClosed   bool        // Writer closed
+	writerChannel  chan string // Writing to the User
+	writerMutex    sync.RWMutex
+	// closeChannel   chan struct{} // Closing
+	wg sync.WaitGroup
+}
+
+func (d *Door) SafeWriterClose() {
+	d.writerMutex.Lock()
+	defer d.writerMutex.Unlock()
+	if !d.WriterClosed {
+		d.WriterClosed = true
+		close(d.writerChannel)
+	}
+}
+
+func (d *Door) WriterIsClosed() bool {
+	d.writerMutex.RLock()
+	defer d.writerMutex.RUnlock()
+	return d.WriterClosed
 }
 
 // Return the amount of time left as time.Duration
@@ -114,7 +131,7 @@ func (d *Door) TimeUsed() time.Duration {
 }
 
 func (d *Door) Disconnect() bool {
-	return atomic.LoadInt32(&d.Disconnected) != 0
+	return d.Disconnected // atomic.LoadInt32(&d.Disconnected) != 0
 }
 
 // Read the BBS door file.  We only support door32.sys.
@@ -290,8 +307,10 @@ 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)
 
 	d.readerChannel = make(chan byte, 8)
-	d.writerChannel = make(chan string)
-	d.closeChannel = make(chan struct{}, 2) // reader & door.Close
+	d.writerChannel = make(chan string) // unbuffered
+
+	// changing this to unbound/sync hangs tests.
+	// d.closeChannel = make(chan struct{}, 2) // reader & door.Close
 
 	d.setupChannels()
 
@@ -315,7 +334,10 @@ func (d *Door) Close() {
 	}()
 
 	log.Println("Closing...")
-	d.closeChannel <- struct{}{}
+	// d.closeChannel <- struct{}{}
+	if !d.WriterClosed {
+		d.writerChannel <- ""
+	}
 
 	// CloseReader(d.Config.Comm_handle)
 

door/help_test.go

@@ -6,6 +6,10 @@ import (
 
 // testing helper routines
 
+func Future_setupSockets() (server net.Conn, client net.Conn) {
+	return net.Pipe()
+}
+
 // setupSockets: return server and client (that's connected to server)
 func setupSockets() (server net.Conn, client net.Conn) {
 	// establish network socket connection to set Comm_handle

door/input.go

@@ -4,7 +4,6 @@ import (
 	"log"
 	"strconv"
 	"strings"
-	"sync/atomic"
 	"time"
 	"unicode"
 )
@@ -79,8 +78,8 @@ func (d *Door) getch() int {
 		if ok {
 			return int(res)
 		} else {
-			// d.Disconnected = true
-			atomic.StoreInt32(&d.Disconnected, 1)
+			d.Disconnected = true
+			// atomic.StoreInt32(&d.Disconnected, 1)
 			return -2
 		}
 	case <-time.After(time.Duration(100) * time.Millisecond):
@@ -268,8 +267,20 @@ func (d *Door) WaitKey(secs int64, usecs int64) int {
 			d.Pushback.Push(int(res))
 			return d.GetKey()
 		} else {
-			// d.Disconnected = true
-			atomic.StoreInt32(&d.Disconnected, 1)
+			// Reader Closed
+
+			d.Disconnected = true
+
+			// If I wrap this with if !d.WriterClosed .. races ?
+
+			// Why can't I do this?  This isn't a go routine...
+			if !d.WriterClosed {
+				d.writerChannel <- ""
+			}
+
+			// d.closeChannel <- struct{}{}
+
+			// atomic.StoreInt32(&d.Disconnected, 1)
 			return -2
 		}
 	case <-time.After(timeout):

door/input_linux.go

@@ -2,7 +2,6 @@ package door
 
 import (
 	"log"
-	"sync/atomic"
 	"syscall"
 )
 
@@ -28,32 +27,38 @@ func Reader(handle int, d *Door) {
 		read, err = syscall.Read(handle, buffer)
 		if err != nil {
 			log.Printf("Reader ERR: %#v\n", err)
+			d.ReaderClosed = true
 			close(d.readerChannel)
-			if !d.Disconnect() {
-				log.Println("Reader close writerChannel")
-				// d.Disconnected = true
-				atomic.StoreInt32(&d.Disconnected, 1)
-				d.closeChannel <- struct{}{}
-				// close(d.writerChannel)
-				return
-			}
-			// break
+			/*
+				if !d.Disconnect() {
+					log.Println("Reader close writerChannel")
+					d.Disconnected = true
+					// atomic.StoreInt32(&d.Disconnected, 1)
+					d.closeChannel <- struct{}{}
+					// close(d.writerChannel)
+					return
+				}
+				// break
+			*/
 			return
 		}
 		if read == 1 {
 			d.readerChannel <- buffer[0]
 		} else {
 			log.Printf("READ FAILED %d\n", read)
+			d.ReaderClosed = true
 			close(d.readerChannel)
-			if !d.Disconnect() {
-				log.Println("Reader close writerChannel")
-				// d.Disconnected = true
-				atomic.StoreInt32(&d.Disconnected, 1)
-				d.closeChannel <- struct{}{}
-				//close(d.writerChannel)
-				return
-			}
-			// break
+			/*
+				if !d.Disconnect() {
+					log.Println("Reader close writerChannel")
+					d.Disconnected = true
+					// atomic.StoreInt32(&d.Disconnected, 1)
+					d.closeChannel <- struct{}{}
+					//close(d.writerChannel)
+					return
+				}
+				// break
+			*/
 			return
 		}
 	}

door/write.go

@@ -126,27 +126,46 @@ res, ok := <-channel
 ok == false
 
 writing to a closed channel is a panic.
+
+Have the Write manage itself.
+if
 */
 
 // Write string to client.
 func (d *Door) Write(output string) {
-	if d.Disconnect() {
+	if output == "" {
 		return
 	}
+	/*
+		if d.Disconnect() {
+			return
+		}
+
+		defer func() {
+			if err := recover(); err != nil {
+				log.Println("Write FAILURE:", err)
+				// Display error to user
+				// This displays stack trace stderr
+				// debug.PrintStack()
+			}
+		}()
 
-	defer func() {
-		if err := recover(); err != nil {
-			log.Println("Write FAILURE:", err)
-			// Display error to user
-			// This displays stack trace stderr
-			// debug.PrintStack()
+		if d.Disconnect() {
+			return
 		}
-	}()
+	*/
 
-	if d.Disconnect() {
-		return
-	}
+	// DATA RACE.  Because WriterClosed is getting changed by
+	// the closeChannel ... this isn't synchronized/safe.
 
-	// Is it possible that the channel would get closed here?
-	d.writerChannel <- output
+	// DATA RACE.  If I just close the writerChannel, there's
+	// a data race on the channel.  (writing to and closing)
+
+	// DATA RACE.  Sending a "" to the channel for a "close"
+	// signal doesn't work either ... (called from go routine)
+	// The channel <- is synced, the access to the bool isn't. :(
+	//if !d.WriterClosed {
+	if !d.WriterIsClosed() {
+		d.writerChannel <- output
+	}
 }

door/write_linux.go

@@ -3,7 +3,6 @@ package door
 import (
 	"log"
 	"strings"
-	"sync/atomic"
 	"syscall"
 )
 
@@ -14,28 +13,45 @@ func Writer(d *Door) {
 	log.Println("Writer")
 	defer d.wg.Done()
 	for {
-		select {
-		case <-d.closeChannel:
-			close(d.writerChannel)
-			log.Println("~Writer")
-			return
-		default:
-		}
+		/*
+			select {
+			case <-d.closeChannel:
+				if !d.WriterClosed {
+					d.WriterClosed = true
+					close(d.writerChannel)
+				}
+				log.Println("~Writer")
+				return
+			default:
+			}
+		*/
 
 		select {
-		case <-d.closeChannel:
-			close(d.writerChannel)
-			log.Println("~Writer")
-			return
+		/*
+			case <-d.closeChannel:
+				// Not safe from data races -- not synced
+				if !d.WriterClosed {
+					d.WriterClosed = true
+					close(d.writerChannel)
+				}
+				// close(d.writerChannel)
+				log.Println("~Writer")
+				return
+		*/
 		case output, ok := <-d.writerChannel:
 			if !ok {
 				log.Println("closeChannel")
-				if !d.Disconnect() {
-					// d.Disconnected = true
-					atomic.StoreInt32(&d.Disconnected, 1)
-					// if !ok, the channel is already closed...
-					// close(d.writerChannel)
-				}
+				// Safe from data races, writerChannel unbuffered
+				d.WriterClosed = true
+				close(d.writerChannel)
+				/*
+					if !d.Disconnect() {
+						// d.Disconnected = true
+						// atomic.StoreInt32(&d.Disconnected, 1)
+						// if !ok, the channel is already closed...
+						// close(d.writerChannel)
+					}
+				*/
 				log.Println("~Writer")
 				return
 
@@ -46,6 +62,19 @@ func Writer(d *Door) {
 				/* Handle cases where we're updating a portion of the screen.
 				When we SavePos, also save/restore the last color set.
 				*/
+				if output == "" {
+					// Close Channel
+					/*
+						if !d.WriterClosed {
+							d.WriterClosed = true
+							close(d.writerChannel)
+						}
+					*/
+					d.SafeWriterClose()
+					log.Println("~Writer")
+					return
+
+				}
 				if strings.HasSuffix(output, RestorePos) {
 					output += Color(d.LastColor...)
 
@@ -57,11 +86,18 @@ func Writer(d *Door) {
 				n, err := syscall.Write(handle, buffer)
 				if (err != nil) || (n != len(buffer)) {
 					log.Println("closeChannel")
-					if !d.Disconnect() {
-						// d.Disconnected = true
-						atomic.StoreInt32(&d.Disconnected, 1)
+					d.SafeWriterClose()
+					/*
+						d.WriterClosed = true
 						close(d.writerChannel)
-					}
+					*/
+					/*
+						if !d.Disconnect() {
+							// d.Disconnected = true
+							// atomic.StoreInt32(&d.Disconnected, 1)
+							close(d.writerChannel)
+						}
+					*/
 					log.Println("~Writer")
 					return
 				}

door32.go

@@ -241,7 +241,7 @@ func Drain(conn net.Conn, drain int) {
 	var err error
 	n, err = conn.Read(buff)
 	if n > 0 {
-		log.Printf("Drained %d bytes.\n", n)
+		log.Printf("Drained %d bytes [%#v].\n", n, buff[:n])
 	}
 	if err != nil {
 		log.Println("Drain:", err)