-
Notifications
You must be signed in to change notification settings - Fork 946
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
Changes from all commits
7958868
6c775cd
f9495a0
519a89d
902c2a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
} |
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. | ||||||
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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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! There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||||||
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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None of this is available on WASI There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||
} |
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 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.