Skip to content

feat: add basic WASI net.Listener via sock_accept #2748

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

Closed
wants to merge 5 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/net/error_posix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// The following is copied and adapted from Go 1.18 official implementation.

// Copyright 2017 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//go:build aix || darwin || dragonfly || freebsd || (js && wasm) || linux || netbsd || openbsd || solaris || windows || wasi
// +build aix darwin dragonfly freebsd js,wasm linux netbsd openbsd solaris windows wasi

package net

import (
"os"
"syscall"
)

// wrapSyscallError takes an error and a syscall name. If the error is
// a syscall.Errno, it wraps it in a os.SyscallError using the syscall name.
func wrapSyscallError(name string, err error) error {
if _, ok := err.(syscall.Errno); ok {
err = os.NewSyscallError(name, err)
}
return err
}
78 changes: 78 additions & 0 deletions src/net/fd_wasi.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//go:build wasi
// +build wasi

package net

import (
"os"
"syscall"
"time"
)

// Network file descriptor.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Network file descriptor.
// wasiCon implements Con with a file returned by syscall.SockAccept.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is directly copy-pasted and adapted from https://github.com/golang/go/blob/a0441c7ae3dea57a0553c9ea77e184c34b7da40f/src/net/fd_posix.go#L16-L17, that's the approach I saw throughout this project so I followed that.

Changing this comment would make it inconsistent with upstream, which means it would not be as trivial to sync this with upstream by copy-pasting.

If we're changing this, then we should change the whole implementation as well, since the way this is designed in the standard library is not necessarily the best approach here given the set of use cases this project tries to cover.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok understand what you mean. I think in another world I would probably comment things like this in the source, but ack that's not the practice. I'll dismiss my feedback as most of them are me being curious why things are defining unused symbols or otherwise. People who know the conventions already somehow won't need to see comments like this.

type netFD struct {
*os.File

net string
laddr Addr
raddr Addr
}

// Read implements the Conn Read method.
func (c *netFD) Read(b []byte) (int, error) {
// TODO: Handle EAGAIN and perform poll
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO: Handle EAGAIN and perform poll
// TODO: Handle EAGAIN and perform poll similar to poll/fd_unix.go

Copy link
Contributor Author

@rvolosatovs rvolosatovs Sep 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, not necessarily same as https://github.com/golang/go/blob/master/src/internal/poll/fd_unix.go
It's circling back to https://github.com/tinygo-org/tinygo/pull/2748/files#r979834379 in my head - is it a goal of this project to be as similar to upstream Go and minimize diff or not? A simpler and more minimal poll implementation could probably suit this project better, but I am not sure what's the preferred approach. I was hoping this PR could start a discussion

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave this one to the maintainers to decide. thanks for clarifying!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the style question, my impression is that the project tries in this order:

  1. directly use upstream files (no copying needed)
  2. borrow a subset of upstream files, minimizing diffs but only borrowing the lines that are needed
  3. replace upstream files with something more suited to tinygo's use case (but still kind of minimizing diffs if possible)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upstream's networking is... tricky because of its interaction with goroutines, as you've noticed.
How useful is networking without getting that bit right?
Will your code (e.g. netfd.Read()) pause all goroutines while blocking on networking?
I haven't checked yet.

return c.File.Read(b)
}

// Write implements the Conn Write method.
func (c *netFD) Write(b []byte) (int, error) {
// TODO: Handle EAGAIN and perform poll
return c.File.Write(b)
}

// LocalAddr implements the Conn LocalAddr method.
func (*netFD) LocalAddr() Addr {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil
return laddr

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not available on WASI

}

// RemoteAddr implements the Conn RemoteAddr method.
func (*netFD) RemoteAddr() Addr {
return nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not available on WASI

}

// SetDeadline implements the Conn SetDeadline method.
func (*netFD) SetDeadline(t time.Time) error {
return ErrNotImplemented
}

// SetReadDeadline implements the Conn SetReadDeadline method.
func (*netFD) SetReadDeadline(t time.Time) error {
return ErrNotImplemented
}

// SetWriteDeadline implements the Conn SetWriteDeadline method.
func (*netFD) SetWriteDeadline(t time.Time) error {
return ErrNotImplemented
}

type listener struct {
*os.File
}

// Accept implements the Listener Accept method.
func (l *listener) Accept() (Conn, error) {
fd, err := syscall.SockAccept(int(l.File.Fd()), syscall.O_NONBLOCK)
if err != nil {
return nil, wrapSyscallError("sock_accept", err)
}
return &netFD{File: os.NewFile(uintptr(fd), "conn"), net: "file+net", laddr: nil, raddr: nil}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a fake addr instead of nil? or comment why? I think usually these are derived from fd, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of this is available on WASI

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll delete all my comments which summarize as "please comment this" :D for now, I guess people will know that if something is nil, it is intentional.

}

// Addr implements the Listener Addr method.
func (*listener) Addr() Addr {
return nil
}

func fileListener(f *os.File) (Listener, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upstream Go does not do this https://github.com/golang/go/blob/a0441c7ae3dea57a0553c9ea77e184c34b7da40f/src/net/file_unix.go#L89-L102

If a structure of the code is confusing, I don't think that explaining everything in comments is the right solution - restructuring the code is, however.
As I said, I tried to keep as close as possible to upstream Go implementation - if that's not a requirement, then I'd rather rewrite this whole thing in a way that makes much more sense especially for this use case

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I would love to see your impl, but for tinygo whatever maintainers suggest is best as they have the long term stake.

return &listener{File: f}, nil
}
31 changes: 31 additions & 0 deletions src/net/file.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//go:build wasi
// +build wasi

// The following is copied from Go 1.18 official implementation.

// Copyright 2015 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package net

import (
"os"
)

type fileAddr string

func (fileAddr) Network() string { return "file+net" }
func (f fileAddr) String() string { return string(f) }

// FileListener returns a copy of the network listener corresponding
// to the open file f.
// It is the caller's responsibility to close ln when finished.
// Closing ln does not affect f, and closing f does not affect ln.
func FileListener(f *os.File) (ln Listener, err error) {
ln, err = fileListener(f)
if err != nil {
err = &OpError{Op: "file", Net: "file+net", Source: nil, Addr: fileAddr(f.Name()), Err: err}
}
return
}
98 changes: 96 additions & 2 deletions src/net/net.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// The following is copied from Go 1.18 official implementation.
// The following is copied and adapted from Go 1.18 official implementation.

// Copyright 2009 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
@@ -8,6 +8,7 @@ package net

import (
"io"
"syscall"
"time"
)

@@ -82,7 +83,100 @@ type Conn interface {
}

type conn struct {
//
fd *netFD
}

func (c *conn) ok() bool { return c != nil && c.fd != nil }

// Implementation of the Conn interface.

// Read implements the Conn Read method.
func (c *conn) Read(b []byte) (int, error) {
if !c.ok() {
return 0, syscall.EINVAL
}
n, err := c.fd.Read(b)
if err != nil && err != io.EOF {
err = &OpError{Op: "read", Net: c.fd.net, Source: c.fd.laddr, Addr: c.fd.raddr, Err: err}
}
return n, err
}

// Write implements the Conn Write method.
func (c *conn) Write(b []byte) (int, error) {
if !c.ok() {
return 0, syscall.EINVAL
}
n, err := c.fd.Write(b)
if err != nil {
err = &OpError{Op: "write", Net: c.fd.net, Source: c.fd.laddr, Addr: c.fd.raddr, Err: err}
}
return n, err
}

// Close closes the connection.
func (c *conn) Close() error {
if !c.ok() {
return syscall.EINVAL
}
err := c.fd.Close()
if err != nil {
err = &OpError{Op: "close", Net: c.fd.net, Source: c.fd.laddr, Addr: c.fd.raddr, Err: err}
}
return err
}

// LocalAddr returns the local network address.
// The Addr returned is shared by all invocations of LocalAddr, so
// do not modify it.
func (c *conn) LocalAddr() Addr {
if !c.ok() {
return nil
}
return c.fd.laddr
}

// RemoteAddr returns the remote network address.
// The Addr returned is shared by all invocations of RemoteAddr, so
// do not modify it.
func (c *conn) RemoteAddr() Addr {
if !c.ok() {
return nil
}
return c.fd.raddr
}

// SetDeadline implements the Conn SetDeadline method.
func (c *conn) SetDeadline(t time.Time) error {
if !c.ok() {
return syscall.EINVAL
}
if err := c.fd.SetDeadline(t); err != nil {
return &OpError{Op: "set", Net: c.fd.net, Source: nil, Addr: c.fd.laddr, Err: err}
}
return nil
}

// SetReadDeadline implements the Conn SetReadDeadline method.
func (c *conn) SetReadDeadline(t time.Time) error {
if !c.ok() {
return syscall.EINVAL
}
if err := c.fd.SetReadDeadline(t); err != nil {
return &OpError{Op: "set", Net: c.fd.net, Source: nil, Addr: c.fd.laddr, Err: err}
}
return nil
}

// SetWriteDeadline implements the Conn SetWriteDeadline method.
func (c *conn) SetWriteDeadline(t time.Time) error {
if !c.ok() {
return syscall.EINVAL
}
if err := c.fd.SetWriteDeadline(t); err != nil {
return &OpError{Op: "set", Net: c.fd.net, Source: nil, Addr: c.fd.laddr, Err: err}
}
return nil
}

// A Listener is a generic network listener for stream-oriented protocols.
Loading