syscall/js.finalizeRef not implemented

[tranxuanthang]

The error happened when I try an example (https://github.com/tinygo-org/tinygo/blob/master/src/examples/wasm/slices/)

Steps to reproduce:

• Clone the tinygo repository git clone git@github.com:tinygo-org/tinygo.git
• cd tinygo/src/examples/wasm/slices
• Build the wasm with tinygo build -o wasm.wasm -target wasm --no-debug ./wasm.go
• Copy the file tinygo/targets/wasm_exec.js to current slices folder
• Run the server, open the address in browser and start typing a number. You should see the error above.

[deadprogram]

That is pretty interesting, @tranxuanthang thanks for reporting.

[prep]

I'm running into the same issue when fetching the name and type properties from File that came out of a FileList (that came out of a DataTransfer).

var val js.Value

// ...

n := val.Get("name").String()
t := val.Get("type").String()

[ssttevee]

I think this is a bigger problem than just annoying log messages.

The purpose of finalizeRef in cgo is to allow the js vm to recover memory held by go. This memory leak issue means, in it's current state, tinygo's wasm target is unfit for anything more than short-lived toy projects.

This probably calls for either a custom implementation of syscall/js where refs are managed differently or just plain implementing runtime.SetFinalizer.

[mehotkhan]

i got this error : syscall/js.finalizeRef not implemented
go version go1.14.6 linux/amd64
tinygo version 0.13.1 linux/amd64 (using go version go1.14.6 and LLVM version 10.0.1)

package main

import (
	"fmt"
	"syscall/js"
	"github.com/buger/jsonparser"
)

func loadSignal(this js.Value, signal []js.Value) interface{} {

	value := signal[0].String()
	data := []byte(value)
	minValue, _ := jsonparser.GetInt(data, "analog", "min")

	return js.ValueOf(minValue)
}

func main() {
	fmt.Println("Hello from wasm")
	js.Global().Set("loadSignal", js.FuncOf(loadSignal))
}

[mehotkhan]

i update Tinygo finalizeRef on wasm_exec.js , from official Go wasm_exec.js
and it seems work , any idea ?

// func finalizeRef(v ref)
"syscall/js.finalizeRef": (sp) => {
    // Note: TinyGo does not support finalizers so this should never be
    // called.
    // :todo : this is copied from main Go wasm_exec
    const id = mem().getUint32(sp + 8, true);
    this._goRefCounts[id]--;
    if (this._goRefCounts[id] === 0) {
        const v = this._values[id];
        this._values[id] = null;
        this._ids.delete(v);
        this._idPool.push(id);
    }
    // console.error('syscall/js.finalizeRef not implemented');
},

[FrankReh]

This error log to the console is going to happen each time a js.Value is converted to sting with the call String().

jsString manually calls finalizeRef because it is trying to cleanup the temporary reference count it is responsible for, so JavaScript can reclaim some of the temporary data Go created and is holding onto. It's the only time finalizeRef is called directly by the Go/wasm code. The other places it is setup to be used go through a SetFinalizer call which is a noop in TinyGo.

jsString had called valuePrepareString (a js function) a few steps earlier. valuePrepareString created a temporariy Uint8Array with a JavaScript string's UTF-8 encoding in it, and via storeValue, gotten a new ref created for it that could be pushed onto the stack. The ref, basically a uint64, is how Go bridges JavaScript's memory model with its own.

jsString uses that ref to get the Uint8Array bytes copied into a Go byte slice and then wants to release the ref (and the Uint8Array) by calling finalizeRef on the ref itself. All other calls to finalizeRef that the Go to JavaScript binding expects to happen would normally be called by the garbage collector at an appropriate time.

The finalizeRef function should be implemented, as was done by @mehotkhan, to avoid this one case of a memory leak. This would be consistent with the form the storeValue already takes in the same file: was_exec.js.

A little more history, in case it helps someone else come up with a nice solution for the bigger problem.

There are two relevant commits. The first from golang in late 2019; the second to TinyGo in April, 2020.

golang/go@54e6ba6

5674c35

As @ssttevee points out, there is a bigger problem.

SetFinalizer is a noop in every version of the garbage collector. Perhaps other TinyGo targets don't rely on a finalizer to release memory. Whenever the Go code wants to access a JavaScript object or function, a ref is created so the Go code can reference it, and causes more JavaScript memory to be allocated and a JavaScript object to be retained. The GC does not release that memory.

[aykevl]

@FrankReh thank you for investigating!

So it appears that the way forward is to implement finalizers in the TinyGo GC?
That's not going to be easy, but probably needs to be done eventually anyway. For most targets, finalizers are not necessary so hopefully they can remain optional.

[FrankReh]

To help avoid any confusion for others finding this thread. The calling convention TinyGo uses, thanks to LLVM's IR calling convention I think, changes the arguments passed to the wasm imported functions. As seen by comparing other imported functions of Go's wasm_exec.js with TinyGo's, the TinyGo versions do not take a stack pointer that is 8 bytes further than the first argument to be pulled off. In many cases, the argument is provided directly as a parameter. In the case of syscall/js.finalizeRef, the value passed to the JavaScript function is the address of the ref value itself on the stack, not the address 8 bytes past the ref value. The earlier version of the function, given above, worked because coincidentally the ref value had been on the stack earlier and at the time the function is called, the ref value appears twice, once at the address given and once 8 bytes further again.

So to avoid confusion, here is, I think, a more proper version of the function. Both versions do the same thing, reading the same valid 'id'.

					// func finalizeRef(v ref)
					"syscall/js.finalizeRef": (v_addr) => {
						// Note: TinyGo does not support finalizers so this is only called
						// for one specific case, by js.go:jsString.
						const id = mem().getUint32(v_addr, true);
						this._goRefCounts[id]--;
						if (this._goRefCounts[id] === 0) {
							const v = this._values[id];
							this._values[id] = null;
							this._ids.delete(v);
							this._idPool.push(id);
						}
					},

This doesn't address the larger memory leak also referred to above.

[siadat]

Having the same issue here and wondering if there are any plans to address the memory leak issue. 🙏

[dyaskur]

any update? I just wondering what will happen if I ignore that error or copy from original wasm_exec?

[FrankReh]

I would still stand by my comment and code snippet from above. There is one place where the finalizeRef is called by the syscall/js.go file so it is a legitimate place to release the JavaScript resource, as my code above does (it comes into play for temporary strings). But it is also true that for the vast majority of js.Value for JavaScript objects, the JavaScript heap will be left with outstanding references in the this._values map that would be considered a memory leak.

So some will find they can ignore the message. And some Wasm binaries may work fine with a smaller number of JavaScript objects that are not released to the JavaScript garbage collector.

I'm not aware of a schedule for a more comprehensive fix.