Leaves terminal in raw mode when calling shell.Close() asynchronously

[matthijskooijman]

When calling shell.Close() when from a goroutine, my terminal becomes unresponsive (echo is disabled) and I have to call "reset" to make it work again.

What I suspect that happens, is that

• run() (indirectly) calls readline, which switches the terminal into raw mode
• run() then waits for a line to be read, or a message on haltChan
• shell.Close() posts a message to haltChan, which makes run() return
• The readline is still running in a goroutine, when the program terminates, leaving the terminal in raw mode.

Given that goroutines that are still running when the program exits are interrupted, without running their defers (AFAIU), this seems something that ishell should fix, not readline. However, it's not entirely clear to me how to tell readline about this. It seems that calling Close() on the readline lib might help (which calls Terminal.Close() which exits raw mode, and also closes stdin, which should cause the reader to return and cleanup).

However, I tried this, and it didn't actually help. I called s.reader.scanner.Close() at the end of Shell.run(), but that seemed to block, or not help. Investigating shows that Terminal.close() also closes stdin, and then waits for its ioloop goroutine to finish, but it seems that the actual stdin read does not return (I would expect it to return an EOF error, though I'm not entirely sure how bufio.Reader.ReadRune(), which is used, is supposed to handle this). Since the ioloop doesn't finish, Terminal.Close() blocks and never gets around to resetting the terminal. I have no more time to dig into this, unfortunately.

[matthijskooijman]

Here's a simple program to reproduce the problem. Start it, and it will exit after a second, leaving the terminal in raw mode.

import "time"
import "gopkg.in/abiosoft/ishell.v2"

func main() {
        shell := ishell.New()

        go func() {
                time.Sleep(1 * time.Second)
                shell.Close()
        }()

        shell.Run()
}

[abiosoft]

Thanks for reporting this. This is definitely not the expected behaviour. Will dig more into it.

[dolanor]

I can confirm this. I have a CLI game client which connects to a server. If I leave the server from a shell command (exit), it is ok. But I have a channel that reacts once the connection is lost. In that case, I don't call the ishell.Context.Stop() but rather the ishell.Shell.Stop() from a goroutine listening on the channel. And there, I have to reset my OS shell to get it back.

[delaneyj]

I'm running into this same issue? Has any one found a work around?

[bjangelo]

I ran into this issue recently and was able to work around it with the following patch to readline.

diff --git a/std.go b/std.go
index 61d44b7..70f477c 100644
--- a/std.go
+++ b/std.go
@@ -193,5 +193,8 @@ func (s *FillableStdin) Read(p []byte) (n int, err error) {
 
 func (s *FillableStdin) Close() error {
        s.stdinBuffer.Close()
+       if closer, ok := s.stdin.(io.Closer); ok {
+               closer.Close()
+       }
        return nil
 }

There is still no synchronization in ishell to wait for the goroutines to exit, but just delaying before program exit with this patch does cause raw mode to exit.

package main

import (
	"time"
	"github.com/abiosoft/ishell"
)

func main() {
	shell := ishell.New()

	go func() {
		time.Sleep(time.Second)
		shell.Close()
	}()

	shell.Run()
	time.Sleep(time.Second)
}

[bjangelo]

There is still no synchronization in ishell to wait for the goroutines to exit, but just delaying before program exit with this patch does cause raw mode to exit.

Because the example calls shell.Close() in a goroutine, which calls s.stop() before s.reader.scanner.Close(). Moving shell.Run() to the goroutine works with the aforementioned readline patch.

package main

import (
	"time"
	"github.com/abiosoft/ishell"
)

func main() {
	shell := ishell.New()

	go func() {
		shell.Run()
	}()

	time.Sleep(time.Second)
	shell.Close()
}