Skip to content
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

syscall/js: deadlock caused by wrapped functions used as browser event handlers using mutex #54328

Closed
atdiar opened this issue Aug 7, 2022 · 3 comments

Comments

@atdiar
Copy link

atdiar commented Aug 7, 2022

What version of Go are you using (go version)?

$ go version 1.18.2

Does this issue reproduce with the latest release?

I haven't installed 1.19 yet.

What operating system and processor architecture are you using (go env)?

linux, amd64

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/atd/.cache/go-build"
GOENV="/home/atd/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/atd/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/atd/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build4134153916=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I am using wasm to create browser event handling callbacks.
These callbacks need to access mutable state in an exclusive manner.
Hence the wrapped functions (run in response to any triggered event) lock the mutex that protect the mutable state.
A critical section should be established for the duration of the callback call (so until the callback returns)

A full reproducer can be found repo link

package main

import (
	"strconv"
	"sync"
	"syscall/js"
        "log
)

// GOOS=js GOARCH=wasm go build -o  server/assets/app.wasm

var mutex = &sync.Mutex{}

// Mutable is a variable that is protected for access by multiple goroutines 
// (we may want to access it asynchronously)
// It is modified in response to user actions in the browser.
// As such, it models the UI tree that would get modified by user initiated events.
// Thus, different event handlers need to take the lock before modifying it
var Mutable int 

var evtcounter = js.Global().Get("document").Call("createElement", "div")

func AddEventListener(event string, target js.Value){
	cb:= js.FuncOf(func(this js.Value, args []js.Value)any{
        // RETRIEVING EVENT INFO
        evt := args[0]
		typ := evt.Get("type").String()
        log.Print("== HANDLING " + typ + " EVENT ==\n")
        
        
        // EVENT HANDLING
        log.Print("LOCKING\n")
		b:= mutex.TryLock() // locking here as we need to access Mutable
        if !b{
            log.Print("ERROR: UNLOCKING phase seems to have been skipped in previous handler. Wrapped Functions are prempted, interleaved?")
            mutex.Lock() // just to cause the deadlock
        }
        log.Print("LOCKED\n")
		Mutable++
		evtcounter.Set("textContent", "# of event triggers: " + strconv.Itoa(Mutable))
        
		evtcounter.Call("focus")

        log.Print("UNLOCKING\n")
		mutex.Unlock() // Unlocking needs to happen before another callback starts.
        log.Print("UNLOCKED ... ready for locking\n")
		return nil
	})
	target.Call("addEventListener", event, cb)
}

func main(){
	
	body:= js.Global().Get("document").Get("body")
	button:= js.Global().Get("document").Call("createElement", "btn")
	button.Set("textContent","click me")
    
    evtcounter.Call("setAttribute", "tabindex","-1") // makes it focusable programmatically


	body.Call("appendChild",evtcounter)
	body.Call("appendChild",button)

	AddEventListener("click",button)
	AddEventListener("focusin",body)


	c := make(chan struct{}, 0)
	<-c
}

What did you expect to see?

I expected to see the mutex being unlocked before any other wasm-based event handling callback can run. Essentially, I was expecting each callback to finish running before the next one.

What did you see instead?

The functions seem to run in an interleaved manner. Which is strange because javascript event handlers should run synchronously, not concurrently.
The lock is not released before another callback runs and attempts to take it resulting in a deadlock.

I tracked the problem to:
eventHandler function in runtime.handleEvent

Which is initialized to be:
syscall/js js.handleEvent()

Which run the wrapped function f.

@atdiar
Copy link
Author

atdiar commented Aug 7, 2022

cc @agnivade @neelance @dr2chase

@neelance
Copy link
Member

neelance commented Aug 7, 2022

You are calling evtcounter.Call("focus"). Maybe this synchronously invokes the focusin event listener?

@atdiar
Copy link
Author

atdiar commented Aug 7, 2022

Oh, that must be it, thanks! I missed that completely.
I thought I could avoid the need for reentrant locks but seems that I am still knocking my head against this.
Sorry for the bother and thanks again.

@atdiar atdiar closed this as completed Aug 7, 2022
@golang golang locked and limited conversation to collaborators Aug 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants