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

runtime: deadlock when calling a windows DLL with a callback #55015

Open
jagobagascon opened this issue Sep 12, 2022 · 3 comments
Open

runtime: deadlock when calling a windows DLL with a callback #55015

jagobagascon opened this issue Sep 12, 2022 · 3 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone

Comments

@jagobagascon
Copy link

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

$ go version
go version go1.19.1 windows/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\aritz\AppData\Local\go-build
set GOENV=C:\Users\aritz\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\aritz\go\bin\pkg\mod
set GONOPROXY=github.com/saltosystems/*
set GONOSUMDB=github.com/saltosystems/*
set GOOS=windows
set GOPATH=C:/Users/aritz/go/bin
set GOPRIVATE=github.com/saltosystems/*
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.19.1
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\projects\bluetooth\go.mod
set GOWORK=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=C:\msys64\tmp\go-build3399795984=/tmp/go-build -gno-record-gcc-switches

What did you do?

I'm working on a library to call WinRT APIs on Windows, and I was trying to remove the need for CGO by replacing all calls to malloc and free by HeapAlloc and HeapFree (kernel32.dll).

Some APIs require callbacks to pass information back to the caller, and we were using CGO to allocate memory into the heap, and have the callbacks defined in C. So we may wait indefinitely for the callback to be called

This was working fine with CGO+malloc. But as soon as we switched to HeapAlloc without CGO, we are getting a fatal error: all goroutines are asleep - deadlock! error.

The problem seems related to #6751, but for some reason it is still failing for me.

This is a bit hard to reproduce, so I created this repo that includes two applications: https://github.com/jagobagascon/go-deadlock-example

They both scan for BLE devices (so you need bluetooth for it to work, if this is a problem I could probably find some other API wher it fails).
But one of them depends on the old malloc+CGO based WinRT API, and the other is pointing to a branch that calls the HeapAlloc function instead (no CGO required).

What did you expect to see?

Changing malloc for HeapAlloc should not cause a deadlock.

What did you see instead?

The same code fails when using HeapAlloc instead of malloc.

Related issues

Maybe #6751

@jagobagascon
Copy link
Author

Just to add a bit more context. The PR that replaces malloc for HeapAlloc in the WinRT library is this: saltosystems/winrt-go#60

Apart from that, both applications that I added as example should behave exactly the same.

@toothrot toothrot changed the title runtime: wrong deadlock when calling a windows DLL with a callback runtime: deadlock when calling a windows DLL with a callback Sep 13, 2022
@toothrot toothrot added this to the Backlog milestone Sep 13, 2022
@toothrot toothrot added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows labels Sep 13, 2022
@mknyszek mknyszek self-assigned this Sep 28, 2022
@prattmic prattmic self-assigned this Sep 28, 2022
@prattmic
Copy link
Member

I agree that this is related to #6751. We look for deadlocks in checkdead, which claims to account for callbacks from syscall.NewCallback. However from my quick look it seems that it will only account for them if a callback has been called at least once prior to checkdead (thus initializing extraMs).

This needs a closer look.

@jagobagascon
Copy link
Author

Hi! This is a friendly ping to check the status of this issue.

We are really looking forward to get rid of CGO in our project and this is blocking us from doing it. We found out that adding a goroutine that stays alive during the execution of our callbacks fixes the issue:

go func() {
	<-time.After(24 * 365 * time.Hour)
}()

But to be honest I would like to avoid using that as a workaround.

deadprogram added a commit to tinygo-org/bluetooth that referenced this issue Sep 17, 2023
Signed-off-by: deadprogram <ron@hybridgroup.com>
deadprogram added a commit to tinygo-org/bluetooth that referenced this issue Sep 17, 2023
Signed-off-by: deadprogram <ron@hybridgroup.com>
deadprogram added a commit to tinygo-org/bluetooth that referenced this issue Sep 17, 2023
Signed-off-by: deadprogram <ron@hybridgroup.com>
jagobagascon added a commit to saltosystems/winrt-go that referenced this issue Sep 20, 2023
There's an error in the Go runtime that causes a deadlock error whenever
we are waiting for a delegate callback to happen. This is because Go is
not able to detect that we are going to receive a syscall in the future.
See this for more information: golang/go#55015
jagobagascon added a commit to saltosystems/winrt-go that referenced this issue Sep 21, 2023
There's an error in the Go runtime that causes a deadlock error whenever
we are waiting for a delegate callback to happen. This is because Go is
not able to detect that we are going to receive a syscall in the future.
See this for more information: golang/go#55015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
Development

No branches or pull requests

5 participants