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@54e6ba6724dfde355070238f9abc16362cac2e3d

5674c35e1477e21eaafab3a51241d84e47a679dd

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.

[dyaskur]

on tinygo 1.18, above code work fine, but on 1.19 why is still return:

panic: runtime error: nil pointer dereference
0002035a:0x13ad Uncaught RuntimeError: unreachable
    at runtime.runtimePanic (<anonymous>:wasm-function[33]:0x13ad)
    at runtime.nilPanic (<anonymous>:wasm-function[15]:0xba3)
    at (io.Reader).Read (<anonymous>:wasm-function[26]:0x10c6)
    at io.ReadAtLeast (<anonymous>:wasm-function[25]:0x10bc)
    at io.ReadFull (<anonymous>:wasm-function[27]:0x10ea)
    at github.com/mervick/aes-everywhere/go/aes256.Encrypt (<anonymous>:wasm-function[99]:0x6f96)
    at syscall/js.handleEvent.resume (<anonymous>:wasm-function[91]:0x5cd0)
    at runtime.scheduler (<anonymous>:wasm-function[64]:0x35ee)
    at resume (<anonymous>:wasm-function[70]:0x3ae1)
    at global.Yaskur._resume (vendor.js:7901)

edit: the problem is similar to https://github.com/tinygo-org/tinygo/issues/1981 , but he said on 1.18 is fail too, but my code worked fine on 1.18. below is my code:

func getCode(this js.Value, i []js.Value) interface{} {

    value1 := i[0].String()

    encrypted := aes256.Encrypt(value1, "Phrase")

    return strings.Replace(encrypted, "U2FsdGVkX1", "", 1)
}

[yaliv]

I got an error too at run, but it's different from @dyaskur's.

panic: syscall/js: call of Value.Get on undefined
mymodule.wasm:0x4e46 Uncaught (in promise) RuntimeError: unreachable
    at runtime._panic.resume (mymodule.wasm:0x4e46)
    at runtime.scheduler (mymodule.wasm:0x8667)
    at _start (mymodule.wasm:0x84b1)
    at _class._callee$ (wasm_exec.tinygo_v0.19.0.mod.js:485)
    at tryCatch (runtime.js:63)
    at Generator.invoke [as _invoke] (runtime.js:294)
    at Generator.next (runtime.js:119)
    at asyncGeneratorStep (asyncToGenerator.js:3)
    at _next (asyncToGenerator.js:25)
    at asyncToGenerator.js:32

Using TinyGo v0.19.0 + modified syscall/js.finalizeRef as suggested by @FrankReh.

Imported packages:

• math/big

[deadprogram]

Is this still an issue with the latest dev branch?

[nikolaydubina]

I have this issue too. Calling String() or js.Value. official go wasm works just fine.

[Forceu]

I have the same problem with tinygo version 0.22.0 linux/amd64 (using go version go1.17.6 and LLVM version 13.0.0)

[FrankReh]

It could be confusing that tinygo hasn't been fixed to implement the js.finalizeRef callback in wasm_exec.js as I showed above could be done. The solution I showed above does help. It avoids the unnecessary warning - which is going to be confusing to anyone who doesn't spend an hour or more digging into this - and more importantly it does release memory the way it should. It's not a very good reminder that there are JavaScript memory being leaked by Tinygo when the js package is being used.

What it doesn't solve is that the runtime garbage collector doesn't actually call js.finalizeRef on memory when it gets reclaimed. So there can still be massive memory leaks which all of us get used to and have our own way of dealing with. But for the one case where the js.go file actually calls js.finalizeRef "manually", it's very nice when it is actually implemented. It's the first thing I change with each new copy of the Tinygo code when there is a new release.

[aykevl]

@FrankReh can you make a PR out of that? It looks like a reasonable solution.

[mjmar01]

I'm experiencing the same issue causing a memory leak and must admit that I'm borderline confused by this issue thread.

For clarification. Does the change described by @FrankReh fix the problem? And has that been implemented in a release or the dev branch?

I've build the dev branch and used that compiler (tinygo version 0.24.0-dev linux/amd64 (using go version go1.16.2 and LLVM version 13.0.1)) but no difference.

[HaoweiCh]

I'm experiencing the same issue

tinygo version 0.23.0 darwin/amd64 (using go version go1.18.2 and LLVM version 14.0.0)

[hollowaykeanho]

Issue seen on tinygo version 0.24.0 linux/amd64 (using go version go1.18.3 and LLVM version 14.0.0). Currently working on an experiment located at https://github.com/hollowaykeanho/ExperimentingGoWASM

This issue is not seen on vanilla go compiler (both go and tinygo are using the same source codes).

EDIT:

I believe it is the String conversion.

[TwiceII]

After reading this thread, I guess there's only one conclusion: the bigger problem that @ssttevee and @FrankReh talk about makes TinyGo WASM... practically unusable? Until that is solved, fixing js/finalizeRef accomplishes nothing.

[ForestJohnson]

Hello, I want to use tinygo to run Golang code in my web browser, and I came across this issue while I was developing my proof of concept application.

I tried out @FrankReh's code in my wasm_exec.js and it did get rid of the error.

I wanted to see how bad this meme-ory leak really is so I conducted a stress test. First I acquired an approximately 4kb text string: https://github.com/lauriro/iotjs-buffer-error-example/blob/main/4kb.txt

var fourKBString = `aaaaa...

Then I made a function which calls itself every 10 milliseconds and passes 8 copies of that string to a golang function. The golang slightly modifies the string and then returns it. so it's passing 32kb of text back and forth between the JS code and the golang code every 10 milliseconds.

  const blah = () => {
    const result = window.age_wasm.test(fourKBString+fourKBString+fourKBString+fourKBString+fourKBString+fourKBString+fourKBString+fourKBString);
    console.log(`result from golang was ${result.length}`);

    setTimeout(blah, 10);
  };

and here is the code on the golang side:

    jsFunction := js.FuncOf(func(this js.Value, args []js.Value) any {
        if len(args) != 1 {
            panic("test() expects exactly 1 argument")
        }
        if args[0].Type() != js.TypeString {
            panic("test() expects its first argument to be a string")
        }
        return fmt.Sprintf("I got: %s", args[0].String())
    })

Here are the results after about 30 thousand iterations, per the firefox memory profiler tool:

The page started at about 2MB of memory used with most of that being taken up by the DOM/strings/the code itself. Then after 5 minutes of spamming every 10 milliseconds, the page was using about 12MB of RAM.

According to my calculator, 30 000 * 32 kilobytes =0.96 gigabytes so it seems clear to me that the entire string is not being leaked each time. TBH I dont really see this kind of memory leak as a real problem for the majority of use cases, it's a very slow leak so as long as this inter-operation between golang and JS is not done 1000s of times per minute, no one will ever notice it is even there.

[evilnoxx]

Hello everyone. I just updated to tinygo 0.23, and the new wasm_exec.js wasn't working with the fix previously posted. I was getting the error TypeError: Cannot convert a BigInt value to a number when invoking mem().getUint32()

We must now call unboxValue() on the v_ref argument.

Here is the fix with the change I'm talking about:

"syscall/js.finalizeRef": (v_ref) => {
    const id = mem().getUint32(unboxValue(v_ref), 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);
    }
},

[EliCDavis]

This issue has been open for 3 years. Is there a roadmap/issue/any other resource people can follow to track when this issue gets resolved? Would love (if possible) a list of bite-sized issues that people can work towards resolving this.

If there's nothing planned, is there actions we can perform to put this on the road map?

[k33g]

@EliCDavis quick'n dirty workaround:

<script>
    const go = new Go() // Go Wasm runtime
    go.importObject.gojs["syscall/js.finalizeRef"] = _ => 0  // 😉

    WebAssembly.instantiateStreaming(fetch("main.wasm"), go.importObject)
        .then(({ instance }) => {
            // foo
        })
        .catch(error => {
            console.log("😡 ouch", error)
        })

</script>

[k33g]

@deadprogram, the current implementation is:

// func finalizeRef(v ref)
"syscall/js.finalizeRef": (v_ref) => {
    // Note: TinyGo does not support finalizers so this should never be
    // called.
    console.error('syscall/js.finalizeRef not implemented');
},

The result is an "error":

Perhaps using console.warn('syscall/js.finalizeRef not implemented');would be less "stressful" 😊:

[deadprogram]

@k33g that sounds like a good idea.

[ldemailly]

As @EliCDavis mentioned, I would also be happy to take a stab if pointed in the right direction / if we turn this into actionable steps?

[ldemailly]

made a PR ^