diamondburned / gotk4

Autogenerated GTK4 bindings for Go
GNU Affero General Public License v3.0
518 stars 20 forks source link

SIGSEGV: segmentation violation when using a go routine inside gtk.DropTarget.Connect #122

Closed SnoutBug closed 10 months ago

SnoutBug commented 10 months ago

I wanted to start a somewhat heavy task (counting files in directories recursively) after the user dropped the directory onto a DropTarget.

For that I use the following code in order for the GUI not to lock up: (for simplicity I hard coded the path for now)

drop.Connect("drop", func(drop *gtk.DropTarget, src *coreglib.Value, x, y float64) {
    go func() {
    numberOfFiles := readDir("/home/me")
    lbl.SetText(fmt.Sprintf("Found %s files", numberOfFiles))
    }()
})

The code for readDir, or simply filepath.Walk

func readDir(url string) uint32 {
    var numberOfFiles uint32

    files, _ := os.ReadDir(url)
    path, _ := filepath.Abs(url)
    for _, f := range files {
        if !f.IsDir() {
            numberOfFiles += 1
        } else {
            numberOfFiles += readDir(filepath.Join(path, f.Name()))
        }
    }
    return numberOfFiles
}

For testing purposes I plugged the code into the dragndrop example you provided. After execution the program will sometimes, but not always crash with:

SIGSEGV: segmentation violation
PC=0x7f48017b8f8c m=5 sigcode=1
signal arrived during cgo execution

goroutine 5 [syscall]:
runtime.cgocall(0x75e2e0, 0xc0000525e0)
    /usr/lib/golang/src/runtime/cgocall.go:157 +0x4b fp=0xc0000525b8 sp=0xc000052580 pc=0x4cd64b
github.com/diamondburned/gotk4/pkg/core/glib._Cfunc__g_is_value(0x144a140)
    _cgo_gotypes.go:970 +0x47 fp=0xc0000525e0 sp=0xc0000525b8 pc=0x594c67
github.com/diamondburned/gotk4/pkg/core/glib.(*value).isValue(...)
    /home/me/go/pkg/mod/github.com/diamondburned/gotk4/pkg@v0.0.6-0.20231227124334-d08a1ac493a4/core/glib/glib.go:971
github.com/diamondburned/gotk4/pkg/core/glib.(*value).unset(0xc00048dfc0)
    /home/me/go/pkg/mod/github.com/diamondburned/gotk4/pkg@v0.0.6-0.20231227124334-d08a1ac493a4/core/glib/glib.go:964 +0x1f fp=0xc000052600 sp=0xc0000525e0 pc=0x59a29f
runtime.call16(0x0, 0x9feec0, 0xc0000a2000, 0x10, 0x10, 0x10, 0xc0000526c0)
    /usr/lib/golang/src/runtime/asm_amd64.s:747 +0x43 fp=0xc000052620 sp=0xc000052600 pc=0x52bf23
runtime.runfinq()
    /usr/lib/golang/src/runtime/mfinal.go:255 +0x3e7 fp=0xc0000527e0 sp=0xc000052620 pc=0x4e0147
runtime.goexit()
    /usr/lib/golang/src/runtime/asm_amd64.s:1650 +0x1 fp=0xc0000527e8 sp=0xc0000527e0 pc=0x52d9c1
created by runtime.createfing in goroutine 1
    /usr/lib/golang/src/runtime/mfinal.go:163 +0x3d

goroutine 1 [syscall, locked to thread]:
runtime.cgocall(0x776ff0, 0xc00017ddd8)
    /usr/lib/golang/src/runtime/cgocall.go:157 +0x4b fp=0xc00017ddb0 sp=0xc00017dd78 pc=0x4cd64b
github.com/diamondburned/gotk4/pkg/gio/v2._Cfunc_g_application_run(0x139e1d0, 0x1, 0x139fbb0)
    _cgo_gotypes.go:13789 +0x4b fp=0xc00017ddd8 sp=0xc00017ddb0 pc=0x5b560b
github.com/diamondburned/gotk4/pkg/gio/v2.(*Application).Run.func3(0xc00017f690?, 0x25?, 0x139fbb0?)
    /home/me/go/pkg/mod/github.com/diamondburned/gotk4/pkg@v0.0.6-0.20231227124334-d08a1ac493a4/gio/v2/gio.go:41611 +0x67 fp=0xc00017de20 sp=0xc00017ddd8 pc=0x5d3607
github.com/diamondburned/gotk4/pkg/gio/v2.(*Application).Run(0xc0001ad470, {0xc000016060?, 0x1, 0x1})
    /home/me/go/pkg/mod/github.com/diamondburned/gotk4/pkg@v0.0.6-0.20231227124334-d08a1ac493a4/gio/v2/gio.go:41611 +0x20b fp=0xc00017dee8 sp=0xc00017de20 pc=0x5d34eb
main.main()
    /home/me/git/gotk4-examples/gtk4/dragndrop/main.go:22 +0xad fp=0xc00017df40 sp=0xc00017dee8 pc=0x75d7ad
runtime.main()
    /usr/lib/golang/src/runtime/proc.go:267 +0x2bb fp=0xc00017dfe0 sp=0xc00017df40 pc=0x500f1b
runtime.goexit()
    /usr/lib/golang/src/runtime/asm_amd64.s:1650 +0x1 fp=0xc00017dfe8 sp=0xc00017dfe0 pc=0x52d9c1

[...]

Is this a bug or am I approaching this issue the wrong way?

diamondburned commented 10 months ago

This code has a race condition: all calls to GTK widgets (lbl.SetText in this case) must be synchronized with glib.IdleAdd, like so:

go func() {
  // blocking call...
  glib.IdleAdd(func() { label.SetText(...) })
}()

Also, prefer using ConnectDrop(...) over Connect("drop", ...).

SnoutBug commented 10 months ago

I think this issue need to be reopened. That does not seem to work.

The app crashed regardless no matter if I used glib.IdleAdd(...) or not. Additionally the app also crashed when I didn't set the label in the go routine. I also tried to include readDir(...) inside glib.IdleAdd(...) which did not work either (that also locks the UI which I wanted to avoid). The app always crashed with the same error.

ConnectDrop(...) does not work as mentioned in #107.

The culprit seems to be os.ReadDir(...): If I put

for {
     os.ReadDir("/home/me")
}

inside either glib.IdleAdd(...) or the go routine directly, the program will crash eventually. It seems like you need to hit the right timing, though.

diamondburned commented 10 months ago

What version of gotk4/pkg are you on? Can you try v0.1.0?

diamondburned commented 10 months ago

Whoops, I put the wrong issue number in there.

diamondburned commented 10 months ago

ConnectDrop should work now, but the latest commit will also include commit 96fbd78af598f729089e8165fc68bed3abb2683e which is a very unstable/experimental commit that I'm not confident will be stable.

If you see any crashes occuring at package intern, please make an issue.

SnoutBug commented 10 months ago

What version of gotk4/pkg are you on?

I think I was on @0.0.6 if I am not mistaken.

Can you try v0.1.0?

I tried @0.1.0 with no success, the app would crash regardless.

I also tried the @4 branch directly, allowing me to use ConnectDrop(...) which worked flawlessly with the go routine, as far as I could tell. However, Connect("drop"...) still did not work.

diamondburned commented 10 months ago

However, Connect("drop"...) still did not work.

This is acceptable. Connect("x") should ideally never be used. I'm only keeping it around just in case it's necessary for some reason.

SnoutBug commented 10 months ago

That works fine for me. Since I can now use the alternative I consider this issue resolved. Thanks a bunch for your help :)