Skip to content

Commit

Permalink
syscall/js: replace TypedArrayOf with CopyBytesToGo/CopyBytesToJS
Browse files Browse the repository at this point in the history
The typed arrays returned by TypedArrayOf were backed by WebAssembly
memory. They became invalid each time we grow the WebAssembly memory.
This made them very error prone and hard to use correctly.

This change removes TypedArrayOf completely and instead introduces
CopyBytesToGo and CopyBytesToJS for copying bytes between a byte
slice and an Uint8Array. This breaking change is still allowed for
the syscall/js package.

Fixes #31980.
Fixes #31812.

Change-Id: I14c76fdd60b48dd517c1593972a56d04965cb272
Reviewed-on: https://go-review.googlesource.com/c/go/+/177537
Run-TryBot: Richard Musiol <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
  • Loading branch information
neelance authored and Richard Musiol committed May 24, 2019
1 parent 7e5bc47 commit c468ad0
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 168 deletions.
29 changes: 28 additions & 1 deletion misc/wasm/wasm_exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,34 @@
mem().setUint8(sp + 24, loadValue(sp + 8) instanceof loadValue(sp + 16));
},

// func copyBytesToGo(dst []byte, src ref) (int, bool)
"syscall/js.copyBytesToGo": (sp) => {
const dst = loadSlice(sp + 8);
const src = loadValue(sp + 32);
if (!(src instanceof Uint8Array)) {
mem().setUint8(sp + 48, 0);
return;
}
const toCopy = src.subarray(0, dst.length);
dst.set(toCopy);
setInt64(sp + 40, toCopy.length);
mem().setUint8(sp + 48, 1);
},

// func copyBytesToJS(dst ref, src []byte) (int, bool)
"syscall/js.copyBytesToJS": (sp) => {
const dst = loadValue(sp + 8);
const src = loadSlice(sp + 16);
if (!(dst instanceof Uint8Array)) {
mem().setUint8(sp + 48, 0);
return;
}
const toCopy = src.subarray(0, dst.length);
dst.set(toCopy);
setInt64(sp + 40, toCopy.length);
mem().setUint8(sp + 48, 1);
},

"debug": (value) => {
console.log(value);
},
Expand All @@ -403,7 +431,6 @@
true,
false,
global,
this._inst.exports.mem,
this,
];
this._refs = new Map();
Expand Down
5 changes: 0 additions & 5 deletions src/crypto/cipher/xor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,11 @@ import (
"crypto/cipher"
"crypto/rand"
"fmt"
"internal/testenv"
"io"
"runtime"
"testing"
)

func TestXOR(t *testing.T) {
if runtime.GOOS == "js" {
testenv.SkipFlaky(t, 31812)
}
for j := 1; j <= 1024; j++ {
if testing.Short() && j > 16 {
break
Expand Down
5 changes: 3 additions & 2 deletions src/crypto/rand/rand_js.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,16 @@ func init() {
}

var jsCrypto = js.Global().Get("crypto")
var uint8Array = js.Global().Get("Uint8Array")

// reader implements a pseudorandom generator
// using JavaScript crypto.getRandomValues method.
// See https://developer.mozilla.org/en-US/docs/Web/API/Crypto/getRandomValues.
type reader struct{}

func (r *reader) Read(b []byte) (int, error) {
a := js.TypedArrayOf(b)
a := uint8Array.New(len(b))
jsCrypto.Call("getRandomValues", a)
a.Release()
js.CopyBytesToGo(b, a)
return len(b), nil
}
18 changes: 8 additions & 10 deletions src/net/http/roundtrip_js.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"syscall/js"
)

var uint8Array = js.Global().Get("Uint8Array")

// jsFetchMode is a Request.Header map key that, if present,
// signals that the map entry is actually an option to the Fetch API mode setting.
// Valid values are: "cors", "no-cors", "same-origin", "navigate"
Expand Down Expand Up @@ -96,9 +98,9 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
return nil, err
}
req.Body.Close()
a := js.TypedArrayOf(body)
defer a.Release()
opt.Set("body", a)
buf := uint8Array.New(len(body))
js.CopyBytesToJS(buf, body)
opt.Set("body", buf)
}
respPromise := js.Global().Call("fetch", req.URL.String(), opt)
var (
Expand Down Expand Up @@ -210,9 +212,7 @@ func (r *streamReader) Read(p []byte) (n int, err error) {
return nil
}
value := make([]byte, result.Get("value").Get("byteLength").Int())
a := js.TypedArrayOf(value)
a.Call("set", result.Get("value"))
a.Release()
js.CopyBytesToGo(value, result.Get("value"))
bCh <- value
return nil
})
Expand Down Expand Up @@ -273,11 +273,9 @@ func (r *arrayReader) Read(p []byte) (n int, err error) {
)
success := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
// Wrap the input ArrayBuffer with a Uint8Array
uint8arrayWrapper := js.Global().Get("Uint8Array").New(args[0])
uint8arrayWrapper := uint8Array.New(args[0])
value := make([]byte, uint8arrayWrapper.Get("byteLength").Int())
a := js.TypedArrayOf(value)
a.Call("set", uint8arrayWrapper)
a.Release()
js.CopyBytesToGo(value, uint8arrayWrapper)
bCh <- value
return nil
})
Expand Down
22 changes: 5 additions & 17 deletions src/syscall/fs_js.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func now() (sec int64, nsec int32)
var jsProcess = js.Global().Get("process")
var jsFS = js.Global().Get("fs")
var constants = jsFS.Get("constants")

var uint8Array = js.Global().Get("Uint8Array")

var (
Expand Down Expand Up @@ -384,10 +385,7 @@ func Read(fd int, b []byte) (int, error) {
if err != nil {
return 0, err
}

a := js.TypedArrayOf(b)
a.Call("set", buf)
a.Release()
js.CopyBytesToGo(b, buf)

n2 := n.Int()
f.pos += int64(n2)
Expand All @@ -406,11 +404,8 @@ func Write(fd int, b []byte) (int, error) {
return n, err
}

a := js.TypedArrayOf(b)
buf := uint8Array.New(len(b))
buf.Call("set", a)
a.Release()

js.CopyBytesToJS(buf, b)
n, err := fsCall("write", fd, buf, 0, len(b), nil)
if err != nil {
return 0, err
Expand All @@ -426,20 +421,13 @@ func Pread(fd int, b []byte, offset int64) (int, error) {
if err != nil {
return 0, err
}

a := js.TypedArrayOf(b)
a.Call("set", buf)
a.Release()

js.CopyBytesToGo(b, buf)
return n.Int(), nil
}

func Pwrite(fd int, b []byte, offset int64) (int, error) {
a := js.TypedArrayOf(b)
buf := uint8Array.New(len(b))
buf.Call("set", a)
a.Release()

js.CopyBytesToJS(buf, b)
n, err := fsCall("write", fd, buf, 0, len(b), offset)
if err != nil {
return 0, err
Expand Down
29 changes: 27 additions & 2 deletions src/syscall/js/js.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ var (
valueTrue = predefValue(3)
valueFalse = predefValue(4)
valueGlobal = predefValue(5)
memory = predefValue(6) // WebAssembly linear memory
jsGo = predefValue(7) // instance of the Go class in JavaScript
jsGo = predefValue(6) // instance of the Go class in JavaScript

objectConstructor = valueGlobal.Get("Object")
arrayConstructor = valueGlobal.Get("Array")
Expand Down Expand Up @@ -478,3 +477,29 @@ type ValueError struct {
func (e *ValueError) Error() string {
return "syscall/js: call of " + e.Method + " on " + e.Type.String()
}

// CopyBytesToGo copies bytes from the Uint8Array src to dst.
// It returns the number of bytes copied, which will be the minimum of the lengths of src and dst.
// CopyBytesToGo panics if src is not an Uint8Array.
func CopyBytesToGo(dst []byte, src Value) int {
n, ok := copyBytesToGo(dst, src.ref)
if !ok {
panic("syscall/js: CopyBytesToGo: expected src to be an Uint8Array")
}
return n
}

func copyBytesToGo(dst []byte, src ref) (int, bool)

// CopyBytesToJS copies bytes from src to the Uint8Array dst.
// It returns the number of bytes copied, which will be the minimum of the lengths of src and dst.
// CopyBytesToJS panics if dst is not an Uint8Array.
func CopyBytesToJS(dst Value, src []byte) int {
n, ok := copyBytesToJS(dst.ref, src)
if !ok {
panic("syscall/js: CopyBytesToJS: expected dst to be an Uint8Array")
}
return n
}

func copyBytesToJS(dst ref, src []byte) (int, bool)
8 changes: 8 additions & 0 deletions src/syscall/js/js_js.s
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,11 @@ TEXT ·valueLoadString(SB), NOSPLIT, $0
TEXT ·valueInstanceOf(SB), NOSPLIT, $0
CallImport
RET

TEXT ·copyBytesToGo(SB), NOSPLIT, $0
CallImport
RET

TEXT ·copyBytesToJS(SB), NOSPLIT, $0
CallImport
RET
74 changes: 52 additions & 22 deletions src/syscall/js/js_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,28 +167,6 @@ func TestFrozenObject(t *testing.T) {
}
}

func TestTypedArrayOf(t *testing.T) {
testTypedArrayOf(t, "[]int8", []int8{0, -42, 0}, -42)
testTypedArrayOf(t, "[]int16", []int16{0, -42, 0}, -42)
testTypedArrayOf(t, "[]int32", []int32{0, -42, 0}, -42)
testTypedArrayOf(t, "[]uint8", []uint8{0, 42, 0}, 42)
testTypedArrayOf(t, "[]uint16", []uint16{0, 42, 0}, 42)
testTypedArrayOf(t, "[]uint32", []uint32{0, 42, 0}, 42)
testTypedArrayOf(t, "[]float32", []float32{0, -42.5, 0}, -42.5)
testTypedArrayOf(t, "[]float64", []float64{0, -42.5, 0}, -42.5)
}

func testTypedArrayOf(t *testing.T, name string, slice interface{}, want float64) {
t.Run(name, func(t *testing.T) {
a := js.TypedArrayOf(slice)
got := a.Index(1).Float()
a.Release()
if got != want {
t.Errorf("got %#v, want %#v", got, want)
}
})
}

func TestNaN(t *testing.T) {
want := js.ValueOf(math.NaN())
got := dummys.Get("NaN")
Expand Down Expand Up @@ -454,3 +432,55 @@ func expectPanic(t *testing.T, fn func()) {
}()
fn()
}

var copyTests = []struct {
srcLen int
dstLen int
copyLen int
}{
{5, 3, 3},
{3, 5, 3},
{0, 0, 0},
}

func TestCopyBytesToGo(t *testing.T) {
for _, tt := range copyTests {
t.Run(fmt.Sprintf("%d-to-%d", tt.srcLen, tt.dstLen), func(t *testing.T) {
src := js.Global().Get("Uint8Array").New(tt.srcLen)
if tt.srcLen >= 2 {
src.SetIndex(1, 42)
}
dst := make([]byte, tt.dstLen)

if got, want := js.CopyBytesToGo(dst, src), tt.copyLen; got != want {
t.Errorf("copied %d, want %d", got, want)
}
if tt.dstLen >= 2 {
if got, want := int(dst[1]), 42; got != want {
t.Errorf("got %d, want %d", got, want)
}
}
})
}
}

func TestCopyBytesToJS(t *testing.T) {
for _, tt := range copyTests {
t.Run(fmt.Sprintf("%d-to-%d", tt.srcLen, tt.dstLen), func(t *testing.T) {
src := make([]byte, tt.srcLen)
if tt.srcLen >= 2 {
src[1] = 42
}
dst := js.Global().Get("Uint8Array").New(tt.dstLen)

if got, want := js.CopyBytesToJS(dst, src), tt.copyLen; got != want {
t.Errorf("copied %d, want %d", got, want)
}
if tt.dstLen >= 2 {
if got, want := dst.Index(1).Int(), 42; got != want {
t.Errorf("got %d, want %d", got, want)
}
}
})
}
}
Loading

0 comments on commit c468ad0

Please sign in to comment.