Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve portability for POSIX systems #99

Closed
wants to merge 1 commit into from
Closed

Conversation

gdamore
Copy link

@gdamore gdamore commented Sep 21, 2015

We use simple CGO style functions for the low level accesses to
set or get termios attributes and the TIOCGWINSIZE ioctl. This
helps for systems where SYS_ioctl may be absent (such as solaris),
while stil keeping POSIX compatibility. (Note however that the
TIOCGWINSIZE call may be missing on some systems. In such a case
we provide a default guess of 80x24, unless environment variables
COLUMNS and LINES indicate otherwise.)

Additionally, there is no need for non-blocking I/O. With Go,
we can use os.Read() in a goroutine, and it will return as soon
as data is available, even if the buffer is not open. This helps
with both portability (O_ASYNC and F_SETOWN aren't universally
available), and readability.

Btw, these changes also allow us to eliminate the use of the
unsafe pointers in the Go code.

I've tested the included demo programs, as well as my own program
on both Darwin and illumos/solaris systems. Furthermore, I've
tested the topsl (github.com/gdamore/topsl) demos with these
changes on both systems.

Note that on solaris this requires the use of go v1.5 or better,
as cgo is not present on older versions of that port. All POSIX
platforms supported by Go 1.5 also support cgo.

We use simple CGO style functions for the low level accesses to
set or get termios attributes and the TIOCGWINSIZE ioctl.  This
helps for systems where SYS_ioctl may be absent (such as solaris),
while stil keeping POSIX compatibility.  (Note however that the
TIOCGWINSIZE call may be missing on some systems. In such a case
we provide a default guess of 80x24, unless environment variables
COLUMNS and LINES indicate otherwise.)

Additionally, there is no need for non-blocking I/O.  With Go,
we can use os.Read() in a goroutine, and it will return as soon
as data is available, even if the buffer is not open.  This helps
with both portability (O_ASYNC and F_SETOWN aren't universally
available), and readability.

Btw, these changes also allow us to eliminate the use of the
unsafe pointers in the Go code.

I've tested the included demo programs, as well as my own program
on both Darwin and illumos/solaris systems.  Furthermore, I've
tested the topsl (github.com/gdamore/topsl) demos with these
changes on both systems.

Note that on solaris this requires the use of go v1.5 or better,
as cgo is not present on older versions of that port.  All POSIX
platforms supported by Go 1.5 also support cgo.
@syohex
Copy link

syohex commented Sep 22, 2015

I think quit channel should be closed in termbox.Close function. If goroutine in termbox.Init() function calls in.Read(buf) and waits input, then quit <- 1 makes goroutine block too, because no goroutine read quit channel. Current implementation, goroutine in termbox.Init() always reads quit channel, so this issue does not happen.

https://github.com/gdamore/termbox-go/blob/a534321a31d4f8ea882d7fcee89c5f0244eb152d/api.go#L93

diff --git a/api.go b/api.go
index dd2423a..41f7494 100644
--- a/api.go
+++ b/api.go
@@ -90,7 +90,7 @@ func Interrupt() {
 // Finalizes termbox library, should be called after successful initialization
 // when termbox's functionality isn't required anymore.
 func Close() {
-   quit <- 1
+   close(quit)
    out.WriteString(funcs[t_show_cursor])
    out.WriteString(funcs[t_sgr0])
    out.WriteString(funcs[t_clear_screen])

@nsf
Copy link
Owner

nsf commented Sep 22, 2015

  1. You can't just throw away SIGIO and claim it behaves the same. The reason why SIGIO was added is because when one goroutine is blocked in an input reading call, the other should be able to Close the termbox context. Sadly closing FD doesn't work on every platform. Some platforms defer an actual closing action until there are no active readers.
  2. The reason to have all that syscall mess is to avoid cgo in the first place. If cgo is not a problem, I would even use my C version of the termbox: github.com/nsf/termbox. But some Go users prefer to keep it that way. Go, amongst other things, is known for being able to create 100% statically linked binaries on linux. A craft which is long forgotten in *nix community with all these LGPL-my-ass libc shared libraries.

If only go had proper build tags support, we could add it as a build variant for systems where provided syscall-based implementations don't work. But at the moment, since syscalls actually cover most of the systems, I would rather prefer having them.

So, I don't know when I'll be able to solve your illumos/Solaris portability problem. Frankly, I want to get rid of SIGIO, but we still need to keep this ability to break the blocking read call. It's possible to implement that via select syscall and pipes. I think select is part of POSIX, but I'm not sure about pipes. The idea is to block input reading goroutine in a select call, while having a pipe which can interrupt it when you send something to it. But I really have no time on that kind of amount of changes. So, my best guess is that maybe during winter holidays I'll be able to work on that. I don't know.

@nsf
Copy link
Owner

nsf commented Sep 22, 2015

Oh, and about the lack of ioctl, how terminals work there then? I mean libc should implement it somehow, if not via ioctl, then how all the tcsetattr stuff works?

@gdamore
Copy link
Author

gdamore commented Sep 22, 2015

On Sep 22, 2015, at 6:54 AM, nsf notifications@github.com wrote:

You can't just throw away SIGIO and claim it behaves the same. The reason why SIGIO was added is because when one goroutine is blocked in an input reading call, the other should be able to Close the termbox context. Sadly closing FD doesn't work on every platform. Some platforms defer an actual closing action until there are no active readers.

What platforms do this? I've never heard of any POSIX platform where this is true.

The reason to have all that syscall mess is to avoid cgo in the first place. If cgo is not a problem, I would even use my C version of the termbox: github.com/nsf/termbox. But some Go users prefer to keep it that way. Go, amongst other things, is known for being able to create 100% statically linked binaries on linux. A craft which is long forgotten in *nix community with all these LGPL-my-ass libc shared libraries.

I share the sentiment of minimalism. However your solution doesn't avoid the problem really but just moves it.

There aren't any platforms where cgo is unavailable (this is new since go 1.5) and my approach creates no additional runtime dependencies apart from libc.

I'm now recalling that go doesn't link against libc on Linux. But on Solaris it does.

If only go had proper build tags support, we could add it as a build variant for systems where provided syscall-based implementations don't work. But at the moment, since syscalls actually cover most of the systems, I would rather prefer having them.

That leaves it unusable for me and my platform. :-(

So, I don't know when I'll be able to solve your illumos/Solaris portability problem. Frankly, I want to get rid of SIGIO, but we still need to keep this ability to break the blocking read call. It's possible to implement that via select syscall and pipes. I think select is part of POSIX, but I'm not sure about pipes. The idea is to block input reading goroutine in a select call, while having a pipe which can interrupt it when you send something to it. But I really have no time on that kind of amount of changes. So, my best guess is that maybe during winter holidays I'll be able to work on that. I don't know.

No syscalls are part of POSIX. Rather the definitions are of C language interfaces where the function is usually a C library function that is a thin wrapper around a system call but can also be implemented in any way.

In Solaris the system call interface is not a public interface. Instead developers are supposed to call the c library.


Reply to this email directly or view it on GitHub.

@gdamore
Copy link
Author

gdamore commented Sep 22, 2015

We have ioctl in Solaris. Just not in the go port. However the termios functions are implemented in the C library on top of ioctl. This is the same for pretty much all platforms.

Sent from my iPhone

On Sep 22, 2015, at 6:56 AM, nsf notifications@github.com wrote:

Oh, and about the lack of ioctl, how terminals work there then? I mean libc should implement it somehow, if not via ioctl, then how all the tcsetattr stuff works?


Reply to this email directly or view it on GitHub.

@nsf
Copy link
Owner

nsf commented Sep 22, 2015

What platforms do this? I've never heard of any POSIX platform where this is true.

Could be darwin (aka MacOSX).

I'm not saying that I don't want to make termbox-go as portable as possible, I'm just saying that I don't want to go with cgo paths on all platforms.The fact that cgo is always available is not an issue. The issue is portability across different glibc versions. Or the lack of glibc as a dependency. Maybe it's not a big deal, but I'd like to keep things that way.

I think we can even do that without special build tags. Let's just make a build which uses cgo on solaris and doesn't use it on all other platforms. But what about SIGIO? Is it unavailable there?

@gdamore
Copy link
Author

gdamore commented Sep 22, 2015

No Darwin behaves normally. I tested this very specifically there.

On Sep 22, 2015, at 7:56 AM, nsf notifications@github.com wrote:

What platforms do this? I've never heard of any POSIX platform where this is true.

Could be darwin (aka MacOSX).

Nope. Darwin works just like you'd expect. If you close a file descriptor outstanding reads will return EBADF.

Btw it would be a tragic bug for any platform to NOT do that. I think there is a fair body of software that may depend on this behavior.

What close can do is wait for inflight writes to complete or fail depending on where they are in the process. My guess is that in the upper part of the stack they would just fail but once the write has been scheduled by buffer cache layer that the close will wait until that completes if the file is marked synchronous. (None of which matters for us. We don't use O_SYNC files. That's for databases and such.)

I'm not saying that I don't want to make termbox-go as portable as possible, I'm just saying that I don't want to go with cgo paths on all platforms.The fact that cgo is always available is not an issue. The issue is portability across different glibc versions. Or the lack of glibc as a dependency. Maybe it's not a big deal, but I'd like to keep things that way.

Yeah I think I understand. Perhaps we can offer a different approach using cgo with my approach for platforms like illumos.

Frankly the go folks need to add these low level functions to their base code if they really believe that avoiding libc is a goal. It's crazy that application developers have to get involved with the kernel system call interface - that should always be abstracted away behind a language call interface. We believe this so strongly with solaris that we've long ago stopped shipping static libc. (This caused some challenges in porting go to Solaris.) The system calls can change without notice in Solaris systems because we only promise a stable c calling interface and we change libc and the kernel in lockstep.

I think we can even do that without special build tags. Let's just make a build which uses cgo on solaris and doesn't use it on all other platforms. But what about SIGIO? Is it unavailable there?

SIGIO is available. But utterly unnecessary. I remain adamant that this approach is flat out wrong. And building select or poll syscalls into go app code is probably extraordinarily problematic as this is one area where I think the various operating systems have a great deal of difference in their system call interfaces. (All hidden by standardizes C library interfaces of course.)


Reply to this email directly or view it on GitHub.

@gdamore
Copy link
Author

gdamore commented Sep 22, 2015

Agreed.

Sent from my iPhone

On Sep 21, 2015, at 11:47 PM, Syohei YOSHIDA notifications@github.com wrote:

I think quit channel should be closed in termbox.Close function. If goroutine in termbox.Init() function calls in.Read(buf) and waits input, then quit <- 1 make goroutine block too, because no goroutine read quit channel. Current implementation, goroutine in termbox.Init() always reads quit channel, so this issue does not happen.

https://github.com/gdamore/termbox-go/blob/a534321a31d4f8ea882d7fcee89c5f0244eb152d/api.go#L93

diff --git a/api.go b/api.go
index dd2423a..41f7494 100644
--- a/api.go
+++ b/api.go
@@ -90,7 +90,7 @@ func Interrupt() {
// Finalizes termbox library, should be called after successful initialization
// when termbox's functionality isn't required anymore.
func Close() {

  • quit <- 1
  • close(quit)
    out.WriteString(funcs[t_show_cursor])
    out.WriteString(funcs[t_sgr0])
    out.WriteString(funcs[t_clear_screen])

    Reply to this email directly or view it on GitHub.

@nsf
Copy link
Owner

nsf commented Sep 22, 2015

f422140

Read the comment in the commit. Are you sure about the fact that it works just fine on darwin? I'm not just making this up.

Although, will test it myself anyway.

@gdamore
Copy link
Author

gdamore commented Sep 22, 2015

So I did some more research. Apparently when using cgo, the C library is
linked against dynamically. This avoids the LGPL mess, and basically makes
cgo generated executables just like any other program built on the system
using only libc.

Since libc must be on the system in order for the system to function at
all, or meet POSIX defined standards, I think this is a quite acceptable
compromise.

To be clear, I completely understand and agree with the intent to avoid
creating dependencies on external software -- dependency and licensing hell
can trivially ensue when you start bringing in a bunch of external things.
However, a dependency on the C library (dynamic mind you, to avoid
licensing hassles caused by LGPL versions of libc) is totally reasonable --
if you're going to avoid that dependency, you might as well also try to
avoid a dependency on the kernel. Its pretty much impossible for an
application program to eliminate all dependencies -- even Go itself
depends on some of these interfaces.

On Tue, Sep 22, 2015 at 8:16 AM, Garrett D'Amore garrett@damore.org wrote:

Agreed.

Sent from my iPhone

On Sep 21, 2015, at 11:47 PM, Syohei YOSHIDA notifications@github.com
wrote:

I think quit channel should be closed in termbox.Close function. If
goroutine in termbox.Init() function calls in.Read(buf) and waits input,
then quit <- 1 make goroutine block too, because no goroutine read quit
channel. Current implementation, goroutine in termbox.Init() always reads
quit channel, so this issue does not happen.

https://github.com/gdamore/termbox-go/blob/a534321a31d4f8ea882d7fcee89c5f0244eb152d/api.go#L93

diff --git a/api.go b/api.go
index dd2423a..41f7494 100644--- a/api.go+++ b/api.go@@ -90,7 +90,7 @@ func Interrupt() {
// Finalizes termbox library, should be called after successful initialization
// when termbox's functionality isn't required anymore.
func Close() {- quit <- 1+ close(quit)
out.WriteString(funcs[t_show_cursor])
out.WriteString(funcs[t_sgr0])
out.WriteString(funcs[t_clear_screen])


Reply to this email directly or view it on GitHub
#99 (comment).

@gdamore
Copy link
Author

gdamore commented Sep 22, 2015

I wrote another test program, which I can attach. It turns out that Darwin
does block in Close(), without waking up the Read() call. This is
strange, but I think this may be a consequence of the /dev/tty driver and
buffering that occurs there rather than files in general. (I'd actually
argue that this is a bug in the darwin tty driver.)

However there is a very nice workaround that avoids the whole SIGIO
problem. If one calls tcflush() just before the close, the read() returns
an error (EBADFD) just like we'd expect. This behavior isn't necessary on
other platforms.

The test program is below. Without the tcflush() the programs works
perfectly on solaris, but on darwin it waits until you press a key before
it returns properly (it does return though!) With the tcflush the darwin
system behaves as you'd expect. Again, this is using POSIX functions, but
the same can be done fairly trivially with syscalls I suppose -- I think
this is a TCFLSH ioctl with arguments fd, 2. (2 means flush both input and
output.) I have no idea if this ioctl is universal -- again this isn't
part of the POSIX standard at all.

package main

import (
"os"
"time"
)

// #include <termios.h>
// #include <sys/ioctl.h>
// int tcsetattr(int fd, int opts, const struct termios *);
// int tcgetattr(int fd, struct termios *);
import "C"

func main() {
f, e := os.Open("/dev/tty")
if e != nil {
println("Cannot open /dev/tty:", e.Error())
os.Exit(1)
}

go func() {
time.Sleep(time.Second * 5)
C.tcflush(C.int(f.Fd()), C.TCIOFLUSH)
println("Waiting for close")
f.Close()
println("Close finished")
}()

data := make([]byte, 128)

for {
n, e := f.Read(data)
if e != nil {
println("Error (done?): ", e.Error())
return
}
println("Got", n, "bytes")
}
}

On Tue, Sep 22, 2015 at 9:04 AM, Garrett D'Amore garrett@damore.org wrote:

So I did some more research. Apparently when using cgo, the C library is
linked against dynamically. This avoids the LGPL mess, and basically makes
cgo generated executables just like any other program built on the system
using only libc.

Since libc must be on the system in order for the system to function at
all, or meet POSIX defined standards, I think this is a quite acceptable
compromise.

To be clear, I completely understand and agree with the intent to avoid
creating dependencies on external software -- dependency and licensing hell
can trivially ensue when you start bringing in a bunch of external things.
However, a dependency on the C library (dynamic mind you, to avoid
licensing hassles caused by LGPL versions of libc) is totally reasonable --
if you're going to avoid that dependency, you might as well also try to
avoid a dependency on the kernel. Its pretty much impossible for an
application program to eliminate all dependencies -- even Go itself
depends on some of these interfaces.

On Tue, Sep 22, 2015 at 8:16 AM, Garrett D'Amore garrett@damore.org
wrote:

Agreed.

Sent from my iPhone

On Sep 21, 2015, at 11:47 PM, Syohei YOSHIDA notifications@github.com
wrote:

I think quit channel should be closed in termbox.Close function. If
goroutine in termbox.Init() function calls in.Read(buf) and waits input,
then quit <- 1 make goroutine block too, because no goroutine read quit
channel. Current implementation, goroutine in termbox.Init() always
reads quit channel, so this issue does not happen.

https://github.com/gdamore/termbox-go/blob/a534321a31d4f8ea882d7fcee89c5f0244eb152d/api.go#L93

diff --git a/api.go b/api.go
index dd2423a..41f7494 100644--- a/api.go+++ b/api.go@@ -90,7 +90,7 @@ func Interrupt() {
// Finalizes termbox library, should be called after successful initialization
// when termbox's functionality isn't required anymore.
func Close() {- quit <- 1+ close(quit)
out.WriteString(funcs[t_show_cursor])
out.WriteString(funcs[t_sgr0])
out.WriteString(funcs[t_clear_screen])


Reply to this email directly or view it on GitHub
#99 (comment).

@gdamore
Copy link
Author

gdamore commented Sep 22, 2015

Just replied. You're right... /dev/tty (not files in general) has a
buffering bug on Darwin. I have found a workaround. tcflush of the input
stream works the magic you need.

On Tue, Sep 22, 2015 at 8:36 AM, nsf notifications@github.com wrote:

f422140
f422140

Read the comment in the commit. Are you sure about the fact that it works
just fine on darwin? I'm not just making this up.

Although, will test it myself anyway.


Reply to this email directly or view it on GitHub
#99 (comment).

@gdamore
Copy link
Author

gdamore commented Jan 9, 2018

I'm no longer interested in pursuing this PR, having long long ago spawned the tcell project which does what I need it to (and more).

@gdamore gdamore closed this Jan 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants