This is almost working with d.Close()

Here's the issue:  I can't shut down the reader go routine.
I have it doing a blocking read from the socket.
I can close it with tests (because I control the underlying socket)
but I can't close it with a normal run.
I'm going to add a bool to the door struct "ReaderShutdown", to
signify that we can expect the readChannel to stop.
Otherwise, let go "stop" it on exit.

Makefile

@@ -1,7 +1,8 @@
 
 all: door32 testdoor testdoor/art.go space-ace/space-ace
 
-testdoor: testdoor/testdoor testdoor/testdoor.exe
+# testdoor: testdoor/testdoor testdoor/testdoor.exe
+testdoor: testdoor/testdoor
 
 font-out: font-out.go
 	go build font-out.go

door/door.go

@@ -31,6 +31,7 @@ import (
 	"sync"
 	"time"
 	"runtime/debug"
+	"sync/atomic"
 )
 
 const SavePos = "\x1b[s"              // Save Cursor Position
@@ -82,7 +83,7 @@ type Door struct {
 	Config        DropfileConfig
 	READFD        int
 	WRITEFD       int
-	Disconnected  bool      // Has User disconnected/Hung up?
+	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
@@ -103,6 +104,10 @@ func (d *Door) TimeUsed() time.Duration {
 	return time.Since(d.StartTime)
 }
 
+func (d *Door) Disconnect() bool {
+	return atomic.LoadInt32(&d.Disconnected) != 0
+}
+
 // Read the BBS door file.  We only support door32.sys.
 func (d *Door) ReadDropfile(filename string) {
 	file, err := os.Open(filename)
@@ -269,9 +274,9 @@ func (d *Door) Init(doorname string) {
 	log.Printf("Loading dropfile %s\n", dropfile)
 	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)
+	d.readerChannel = make(chan byte, 8)
 	d.writerChannel = make(chan string)
-	d.closeChannel = make(chan bool,5)
+	d.closeChannel = make(chan bool, 2) // reader & door.Close
 
 	d.setupChannels()
 
@@ -294,11 +299,21 @@ func (d *Door) Close() {
                 }
         }()
 
-	log.Printf("Closing...")
+	log.Println("Closing...")
 	d.closeChannel <- true
+	if ! d.Disconnect() {
+		// log.Println("close write channel")
+		// close(d.writerChannel)
+		log.Println("close read handle")
+		CloseReader(d.Config.Comm_handle)
+		log.Println("closed read handle")
+	}
+
+	// d.closeChannel <- true
 	// close(d.closeChannel)
+	log.Println("wg.Wait()")
 	d.wg.Wait()
-	log.Printf("Closed.")
+	log.Println("Closed.")
 }
 
 // Goto X, Y - Position the cursor using ANSI Escape Codes

door/door_linux.go

@@ -1,8 +1,9 @@
 package door
 
 func (d *Door) setupChannels() {
-	go Reader(d.Config.Comm_handle, &d.readerChannel)
+	d.wg.Add(2)
+	go Reader2(d.Config.Comm_handle, d)
+	// go Reader(d.Config.Comm_handle, &d.readerChannel)
 	// go Writer(d.Config.Comm_handle, &d.writerChannel)
 	go Writer2(d.Config.Comm_handle, d)
-	(*d).wg.Add(1)
 }

door/door_test.go

@@ -163,6 +163,7 @@ func TestDetectFail(t *testing.T) {
 
 	// Send nothing
 	var fd int = socket_to_fd(client)
+	defer close_fd(fd)
 
 	// Create door32.sys file
 	dfc := DropfileConfig{2, fd, 1800, "Test BBSID", 1701, "Real Username", "Handle", 880, 28, 0, 13}

door/help_linux_test.go

@@ -2,10 +2,29 @@ package door
 
 import (
 	"net"
+	"os"
 )
 
+var fdmap map[uintptr] *os.File
+
+func init() {
+	fdmap = make(map[uintptr] *os.File)
+}
+
 func socket_to_fd(socket net.Conn) int {
 	client_conn := socket.(*net.TCPConn)
+	// This creates a duplicate, but once closed -- the fd gets reused!
 	client_file, _ := client_conn.File()
+	fdmap[client_file.Fd()] = client_file
 	return int(client_file.Fd())
 }
+
+func close_fd(fd int) {
+	var fd_uintptr uintptr = uintptr(fd)
+	file, ok := fdmap[fd_uintptr]
+	if ok {
+		file.Close()
+		delete(fdmap, fd_uintptr)
+	}
+}
+

door/input.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 	"time"
 	"unicode"
+	"sync/atomic"
 )
 
 // This is the current list of Extended keys we support:
@@ -45,7 +46,8 @@ func (d *Door) getch() int {
 		if ok {
 			return int(res)
 		} else {
-			d.Disconnected = true
+			// d.Disconnected = true
+			atomic.StoreInt32(&d.Disconnected, 1)
 			return -2
 		}
 	case <-time.After(time.Duration(100) * time.Millisecond):
@@ -74,7 +76,7 @@ func (d *Door) getkey_or_pushback() int {
 func (d *Door) GetKey() int {
 	var c, c2 int
 
-	if d.Disconnected {
+	if d.Disconnect() {
 		return -2
 	}
 
@@ -260,7 +262,7 @@ func (d *Door) Key() int {
 
 // usecs = Microseconds
 func (d *Door) WaitKey(secs int64, usecs int64) int {
-	if d.Disconnected {
+	if d.Disconnect() {
 		return -2
 	}
 
@@ -276,7 +278,8 @@ func (d *Door) WaitKey(secs int64, usecs int64) int {
 			d.Pushback.Push(int(res))
 			return d.GetKey()
 		} else {
-			d.Disconnected = true
+			// d.Disconnected = true
+			atomic.StoreInt32(&d.Disconnected, 1)
 			return -2
 		}
 	case <-time.After(timeout):

door/input_linux.go

@@ -3,8 +3,58 @@ package door
 import (
 	"log"
 	"syscall"
+	"sync/atomic"
 )
 
+func Reader2(handle int, d *Door) {
+	// I don't need the select anymore.  Let the read block.
+	// defer d.wg.Done()
+	defer func() {
+		log.Printf("~Reader2\n")
+		d.wg.Done()
+	}()
+	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(d.readerChannel)
+			if ! d.Disconnect() {
+				log.Println("Reader close writerChannel")
+				// d.Disconnected = true
+				atomic.StoreInt32(&d.Disconnected, 1)
+				d.closeChannel <- true
+				// close(d.writerChannel)
+				return
+			}
+			// break
+			return
+		}
+		if read == 1 {
+			d.readerChannel <- buffer[0]
+		} else {
+			log.Printf("READ FAILED %d\n", read)
+			close(d.readerChannel)
+			if ! d.Disconnect() {
+				log.Println("Reader close writerChannel")
+				// d.Disconnected = true
+				atomic.StoreInt32(&d.Disconnected, 1)
+				d.closeChannel <- true
+				//close(d.writerChannel)
+				return
+			}
+			// break
+			return
+		}
+	}
+}
+
 func Reader(handle int, readerChannel *chan byte) {
 	// I don't need the select anymore.  Let the read block.
 	defer func() {
@@ -15,6 +65,8 @@ func Reader(handle int, readerChannel *chan byte) {
 
 	buffer := make([]byte, 1)
 	for {
+		// blocking read in go routine
+		// why doesn't this end when I close the handle?
 		read, err := syscall.Read(handle, buffer)
 		if err != nil {
 			log.Printf("Reader ERR: %#v\n", err)
@@ -32,5 +84,11 @@ func Reader(handle int, readerChannel *chan byte) {
 }
 
 func CloseReader(handle int) {
+	defer func() {
+		if err := recover(); err != nil {
+			log.Printf("CloseReader: %#v\n", err)
+		}
+	}()
+
 	syscall.Close(handle)
 }

door/input_test.go

@@ -31,6 +31,7 @@ func TestDoorInputConnection(t *testing.T) {
 
 	// Access Fd (File descriptor) of client for dropfile
 	var fd int = socket_to_fd(client)
+	defer close_fd(fd)
 
 	// Create door32.sys file
 	dfc := DropfileConfig{2, fd, 1800, "Test BBSID", 1701, "Real Username", "Handle", 880, 28, 0, 12}
@@ -224,8 +225,8 @@ func TestDoorInputConnection(t *testing.T) {
 		t.Errorf("Expected -2 (hangup), got %d", hungup)
 	}
 
-	if !d.Disconnected {
-		t.Errorf("Disconnected flag shows: %t (should be true)", d.Disconnected)
+	if !d.Disconnect() {
+		t.Errorf("Disconnected flag shows: %t (should be true)", d.Disconnect())
 	}
 
 	/*

door/menu_test.go

@@ -69,6 +69,7 @@ func TestMenuConnection(t *testing.T) {
 
 	// Access Fd (File descriptor) of client for dropfile
 	var fd int = socket_to_fd(client)
+	defer close_fd(fd)
 
 	// Create door32.sys file
 	dfc := DropfileConfig{2, fd, 1800, "Test BBSID", 1701, "Real Username", "Handle", 880, 28, 0, 12}
@@ -140,7 +141,7 @@ func TestMenuConnection(t *testing.T) {
 	server.Write([]byte(keys))
 	time.Sleep(time.Millisecond)
 	runtime.Gosched()
-	if d.Disconnected {
+	if d.Disconnect() {
 		t.Errorf("Disconnected")
 	}
 
@@ -182,7 +183,7 @@ func TestMenuConnection(t *testing.T) {
 
 	time.Sleep(time.Millisecond)
 	runtime.Gosched()
-	if d.Disconnected {
+	if d.Disconnect() {
 		t.Errorf("Disconnected")
 	}
 
@@ -221,7 +222,7 @@ func TestMenuConnection(t *testing.T) {
 	}
 
 	time.Sleep(time.Millisecond)
-	if d.Disconnected {
+	if d.Disconnect() {
 		t.Errorf("Disconnected")
 	}
 

door/nomoresecrets.go

@@ -231,6 +231,9 @@ func NoMoreSecrets(output string, Door *Door, config *NoMoreSecretsConfig) {
 				}
 			}
 			Door.Write(renderF())
+			if Door.Disconnect() {
+				return
+			}
 			time.Sleep(time.Millisecond * time.Duration(config.Jumble_Loop_Speed))
 		}
 
@@ -266,6 +269,9 @@ func NoMoreSecrets(output string, Door *Door, config *NoMoreSecretsConfig) {
 			}
 
 			Door.Write(renderF())
+			if Door.Disconnect() {
+				return
+			}
 			time.Sleep(time.Millisecond * time.Duration(config.Reveal_Loop_Speed))
 			if revealed {
 				break
@@ -400,6 +406,9 @@ func NoMoreSecrets(output string, Door *Door, config *NoMoreSecretsConfig) {
 				}
 			}
 			Door.Write(renderF())
+			if Door.Disconnect() {
+				return
+			}
 			time.Sleep(time.Millisecond * time.Duration(config.Jumble_Loop_Speed))
 		}
 
@@ -431,6 +440,9 @@ func NoMoreSecrets(output string, Door *Door, config *NoMoreSecretsConfig) {
 			}
 
 			Door.Write(renderF())
+			if Door.Disconnect() {
+				return
+			}
 			time.Sleep(time.Millisecond * time.Duration(config.Reveal_Loop_Speed))
 			if revealed {
 				break

door/write.go

@@ -128,45 +128,27 @@ ok == false
 writing to a closed channel is a panic.
 */
 
-// var writeMutex sync.Mutex
 
 // Write string to client.
 func (d *Door) Write(output string) {
-	if d.Disconnected {
+	if d.Disconnect() {
 		return
 	}
 
-	d.writerChannel <- output
-	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 err := recover(); err != nil {
+                        log.Println("Write FAILURE:", err)
+                        // Display error to user
+                        // This displays stack trace stderr
+                        // debug.PrintStack()
+                }
+        }()
 
-	defer func() {
-		if r := recover(); r != nil {
-			log.Println("Write error/HANGUP.")
-			d.Disconnected = true
-		}
-	}()
-
-	// Updating some other part of the screen.
-	// Use SavePos/RestorePos in the same string.
-	// We'll append the LastColor to the string,
-	// allowing the update to not mess up current color.
-	if strings.HasSuffix(output, RestorePos) {
-		output += Color(d.LastColor...)
-	} else {
-		d.UpdateLastColor(output, &d.LastColor)
+	if d.Disconnect() {
+		return
 	}
 
-	/*
-		// Output the string, translating the ESC code to ^[ for outputting.
-		temp := strings.Replace(output, "\x1b", "^[", -1)
-		log.Printf("Parse: [%s]\n", temp)
-	*/
-
+	// Is it possible that the channel would get closed here?
 	d.writerChannel <- output
+	return
 }

door/write_linux.go

@@ -4,34 +4,39 @@ import (
 	"log"
 	"strings"
 	"syscall"
+	"sync/atomic"
 )
 
 // https://go101.org/article/channel-closing.html
+
+// This will get renamed back to Writer, once I get
+// it working...
 func Writer2(handle int, d *Door) {
 	log.Println("Writer2")
 	defer d.wg.Done()
 	for {
 		select {
-		case <-d.closeChannel:
-			// close((*d).writerChannel)
-			log.Println("~Writer2")
-			return
+		case <- d.closeChannel:
+		close(d.writerChannel)
+		log.Println("~Writer2")
+		return
 		default:
 		}
 
 		select {
-		case <-d.closeChannel:
-			// close((*d).writerChannel)
-			log.Println("~Writer2")
-			return
-
+                case <- d.closeChannel:
+		close(d.writerChannel)
+		log.Println("~Writer2")
+		return
 		case output, ok := <-d.writerChannel:
 			if !ok {
-				// (*d).writerChannel = nil
 				log.Println("closeChannel")
-				// close(d.closeChannel)
-				d.Disconnected = 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("~Writer2")
 				return
 
@@ -39,6 +44,9 @@ func Writer2(handle int, d *Door) {
 //				continue
 			} else {
 				// log.Println("output")
+				/* Handle cases where we're updating a portion of the screen.
+				When we SavePos, also save/restore the last color set.
+				*/
 				if strings.HasSuffix(output, RestorePos) {
 					output += Color(d.LastColor...)
 
@@ -49,16 +57,14 @@ func Writer2(handle int, d *Door) {
 				buffer := []byte(output)
 				n, err := syscall.Write(handle, buffer)
 				if (err != nil) || (n != len(buffer)) {
-					// close((*d).writerChannel)
-					// (*d).writerChannel = nil
 					log.Println("closeChannel")
-					// close(d.closeChannel)
-					d.Disconnected = true
+					if ! d.Disconnect() {
+					// d.Disconnected = true
+					atomic.StoreInt32(&d.Disconnected, 1)
 					close(d.writerChannel)
+					}
 					log.Println("~Writer2")
 					return
-//					d.closeChannel <- true
-//					continue
 				}
 			}
 		}

testdoor/testdoor.go

@@ -397,7 +397,7 @@ func progress_bars(d *door.Door) {
 		update_bars()
 		d.Write(bar_start + bar.Output() + "  " + door.Reset + bar2.Output() + door.Reset + "  " + bar3.Output())
 
-		if d.Disconnected {
+		if d.Disconnect() {
 			// don't continue to sleep if we're disconnected
 			break
 		}
@@ -525,7 +525,7 @@ func main() {
 		}
 	}()
 
-	defer d.Close()
+	// defer d.Close()
 	ticker := time.NewTicker(time.Second)
 
 	go func() {
@@ -535,7 +535,12 @@ func main() {
 
 			// maxlen = 12 + 7 + 5 = 24
 			output := door.SavePos + door.Goto(door.Width-25, 0) + door.ColorText("BRI WHI ON BLUE") + " " + t.Format(tf) + " " + timeinfo + " " + door.RestorePos
-			d.Write(output)
+			if !d.Disconnect() {
+				d.Write(output)
+			} else {
+				ticker.Stop()
+				return
+			}
 		}
 	}()