api.go: don't use multiple descriptors to work with /dev/tty (BSD SIGIO TTY)

[mbelop]

Hi,

I'm investigating an issue on OpenBSD where termbox programs that use functionality similar to the editbox demo program lock up on start and never receive SIGIO that lets it progress any further.

The issue turns out to be a combination of unfortunate circumstances within the BSD kernel tty code, Go libraries and termbox design decisions. This is going to be a somewhat long explanation, so please bear with me. First of all what we're looking at is this sequence of actions:

    out, err = os.OpenFile("/dev/tty", syscall.O_WRONLY, 0)
    in, err = syscall.Open("/dev/tty", syscall.O_RDONLY, 0)
    _, err = fcntl(in, syscall.F_SETFL, syscall.O_ASYNC|syscall.O_NONBLOCK)
    err = tcgetattr(out.Fd(), &orig_tios)

found at https://github.com/nsf/termbox-go/blob/4ed959e0540971545eddb8c75514973d670cf739/api.go#L51

What happens here is that two file descriptors are created in the kernel that correspond to the same underlying character device driver (/dev/tty). After the first fcntl call, the flags of the in descriptor contain the FASYNC flag and the very moment the system call is made the driver is switched over to the asynchronous mode and will deliver SIGIO on input. So far so good, however, the next thing done here is the dreaded out.Fd() call and since about 7 months and https://github.com/golang/go/commit/c05b06a12d005f50e4776095a60d6bd9c2c91fac commit Go does the following thing:

https://github.com/golang/go/blob/71c9454f99791d1347996b61797d9d497a0b2942/src/os/file_unix.go#L61

Which ends up doing F_GETFL/F_SETFL sequence every time it's called:

https://github.com/golang/go/blob/71c9454f99791d1347996b61797d9d497a0b2942/src/syscall/exec_unix.go#L98

But since it's called on the out that doesn't have FASYNC set, the kernel switches asynchronous mode off for the tty driver since both in and out correspond to the same device but don't share FASYNC (or other) flags.

OpenBSD kernel source code for reference (FreeBSD does basically the same): https://github.com/openbsd/src/blob/d20f7f0ada2a0a73d7fbf17ce8fb97af3507f19f/sys/kern/kern_descrip.c#L420 https://github.com/openbsd/src/blob/d20f7f0ada2a0a73d7fbf17ce8fb97af3507f19f/sys/kern/tty.c#L753

There's no easy way to fix this as this relies on too many variables to work and as we've seen with that Go commit they can change without an advance notice. BSD behavior has been around for ages and I can't come up with a decent fix right now (even bearing in mind that this "fix" can break a ton of software relying on this.) I suggest avoid using out.Fd() somehow or better don't use multiple descriptors to work with /dev/tty.

Cc @martelletto

[nsf]

I think we should somehow stop relying on SIGIO. But I don't have any ideas right now, maybe will have a look at it next weekend. It is also scary to change termbox's core code, high chance to break things for a lot of users.

[mbelop]

Yes, SIGIO is a terrible mechanic. In the meantime I managed to get input working with this:

    out, err = os.OpenFile("/dev/tty", syscall.O_RDWR, 0)
    if err != nil {
        return err
    }
    in = int(out.Fd())

There still appears to be input issues in the program I'm using but they might be unrelated. I'll keep you posted.

[nsf]

Well, I'm thinking about a way to get rid of SIGIO. But the reason why it was introduced is to handle interruption properly somehow. With SIGIO we don't block on a read, but instead we block waiting for a signal and hope that Go signal lib handles this well. The only alternative I can think of which is also reasonable portable is to use select() and pipes. And that's how termbox C version works: https://github.com/nsf/termbox/blob/master/src/termbox.c#L653. Well, it actually doesn't support interruption, but SIGWINCH handler sends a messag to a pipe which is then dispatched to a common "select" call. In Go of course we have "select" statement and we use it instead. But maybe it's not a bad idea after all. Create an interruptable reader which just does a normal blocking select call on an FD of interest and a pipe through which you can send an "interrupt" message. And so this way it might work. But definitely needs testing. I recall a lot of problems with it all.

What makes it worse is that I switched from an intel cpu to an amd cpu recently, having hard time running macosx in a VM. So, I think I need some time to solve that problem and when I have a few operating systems that I can test termbox on, I can try this pipe-based approach and see what kind of problems does it have.

[marcopeereboom]

I kind of like the idea of the multi-select. That fits nicely with the go idioms.

[dajohi]

Any news?

[jrick]

Would you be willing to implement a workaround for this?

if runtime.GOOS == "openbsd" {
  // openbsd fix
} else {
  // as before
}

[dajohi]

        if runtime.GOOS == "openbsd" {
                out, err = os.OpenFile("/dev/tty", os.O_RDWR, 0)
                if err != nil {
                        return err
                }
                in = int(out.Fd())
        } else {
                out, err = os.OpenFile("/dev/tty", syscall.O_WRONLY, 0)
                if err != nil {
                        return err
                }
                in, err = syscall.Open("/dev/tty", syscall.O_RDONLY, 0)
                if err != nil {
                        return err
                }
        }