search

markus-wa / demoinfocs-golang

A Counter-Strike 2 & CS:GO demo parser for Go (demoinfo)
https://pkg.go.dev/github.com/markus-wa/demoinfocs-golang/v4/pkg/demoinfocs?tab=doc
MIT License
716 stars 95 forks source link

deadlock when calling Parser.Cancel() multiple times #276

Closed Julien2313 closed 3 years ago

Julien2313 commented 3 years ago

Hello :) While parsing a demo, if a use the function Cancel() or Close(), the parser panics with a deadlock error

Code:

package main

import (
    "bufio"
    "bytes"
    "compress/bzip2"
    "compress/gzip"
    "fmt"
    "io"
    "io/ioutil"
    "os"
    "path/filepath"

    "github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs"
    "github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs/events"
)

func main() {
    demoFile, err := os.Open("./pathToDemo.dem")
    if err != nil {
        panic(err)
    }
    defer demoFile.Close()

    var demoUnzippedReader io.Reader
    switch filepath.Ext(demoFile.Name()) {
    case ".bz2":
        demoUnzippedReader = bzip2.NewReader(bufio.NewReader(demoFile))
    case ".gz":
        demoZippedReader, err := gzip.NewReader(demoFile)
        if err != nil {
            panic(err)
        }

        t, err := ioutil.ReadAll(demoZippedReader)
        if err != nil {
            panic(err)
        }
        demoUnzippedReader = bytes.NewReader(t)
    case ".dem":
        demoUnzippedReader = bufio.NewReader(demoFile)
    default:
        panic(err)
    }

    demoUnzippedBytes, err := ioutil.ReadAll(demoUnzippedReader)
    if err != nil {
        panic(err)
    }

    r := bytes.NewReader(demoUnzippedBytes)
    p := demoinfocs.NewParser(r)
    defer p.Close()

    // Register handler on kill events
    p.RegisterEventHandler(func(e events.Kill) {
        fmt.Println("kill !")
        p.Close() // or p.Cancel()
    })

    _, err = p.ParseHeader()
    if err != nil {
        panic(err)
    }
    // Parse to end
    err = p.ParseToEnd()
    if err != nil {
        panic(err)
    }
}

julien@julien-VirtualBox:~/git/test-demoinfo$ go run .
kill !
fatal error: all goroutines are asleep - deadlock!

goroutine 1 [chan send]:
github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs.(*parser).parsePacket(0xc0000dc9a0)
        /home/julien/go/pkg/mod/github.com/markus-wa/demoinfocs-golang/v2@v2.6.0/pkg/demoinfocs/parsing.go:348 +0x2e6
github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs.(*parser).parseFrame(0xc0000dc9a0, 0xc000030060)
        /home/julien/go/pkg/mod/github.com/markus-wa/demoinfocs-golang/v2@v2.6.0/pkg/demoinfocs/parsing.go:257 +0x105
github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs.(*parser).ParseToEnd(0xc0000dc9a0, 0x0, 0x0)
        /home/julien/go/pkg/mod/github.com/markus-wa/demoinfocs-golang/v2@v2.6.0/pkg/demoinfocs/parsing.go:119 +0xd7
main.main()
        /home/julien/git/test-demoinfo/main.go:66 +0x41f

goroutine 20 [semacquire]:
sync.runtime_Semacquire(0xc0004ea200)
        /usr/local/go/src/runtime/sema.go:56 +0x45
sync.(*WaitGroup).Wait(0xc0004ea1f8)
        /usr/local/go/src/sync/waitgroup.go:130 +0x65
github.com/markus-wa/godispatch.(*Dispatcher).sendToken(0xc0004ea1e0, 0xc00000e028, 0x1, 0x1, 0x84a4e0, 0x90b5b8, 0x0, 0x0)
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:266 +0xda
github.com/markus-wa/godispatch.(*Dispatcher).RemoveQueues(...)
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:200
github.com/markus-wa/godispatch.(*Dispatcher).RemoveAllQueues(...)
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:205
github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs.(*parser).Close(0xc0000dc9a0)
        /home/julien/go/pkg/mod/github.com/markus-wa/demoinfocs-golang/v2@v2.6.0/pkg/demoinfocs/parser.go:241 +0x65
main.main.func1(0xc0021ba180, 0xc0021b4b60, 0xc0021b4820, 0x0, 0x0, 0x1, 0x40e01bc8)
        /home/julien/git/test-demoinfo/main.go:58 +0x9f
reflect.Value.call(0x849e20, 0xc00000c180, 0x13, 0x8e1387, 0x4, 0xc0028d7998, 0x1, 0x1, 0xc0000d0c60, 0x0, ...)
        /usr/local/go/src/reflect/value.go:476 +0x8e7
reflect.Value.Call(0x849e20, 0xc00000c180, 0x13, 0xc0028d7998, 0x1, 0x1, 0xc002aa8de8, 0x4a64eb2107f9ac01, 0x4a00000003000106)
        /usr/local/go/src/reflect/value.go:337 +0xb9
github.com/markus-wa/godispatch.callConsumerCode(0x849e20, 0xc00000c180, 0x13, 0xc0028d7998, 0x1, 0x1)
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:123 +0x8c
github.com/markus-wa/godispatch.(*Dispatcher).Dispatch(0xc0004ea240, 0x8c8920, 0xc0021ba600)
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:109 +0x258
github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs.gameEventHandler.dispatch(...)
        /home/julien/go/pkg/mod/github.com/markus-wa/demoinfocs-golang/v2@v2.6.0/pkg/demoinfocs/game_events.go:60
github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs.gameEventHandler.playerDeath(0xc0000dc9a0, 0x0, 0xc0000a1110, 0xc0000a1140, 0xc002f568d0)
        /home/julien/go/pkg/mod/github.com/markus-wa/demoinfocs-golang/v2@v2.6.0/pkg/demoinfocs/game_events.go:336 +0x5e5
github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs.newGameEventHandler.func2.1(0xc002f568d0)
        /home/julien/go/pkg/mod/github.com/markus-wa/demoinfocs-golang/v2@v2.6.0/pkg/demoinfocs/game_events.go:102 +0x62
github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs.(*parser).handleGameEvent(0xc0000dc9a0, 0xc001ee5900)
        /home/julien/go/pkg/mod/github.com/markus-wa/demoinfocs-golang/v2@v2.6.0/pkg/demoinfocs/game_events.go:39 +0x182
reflect.Value.call(0x8499e0, 0xc00001e070, 0x13, 0x8e1387, 0x4, 0xc0028d7f28, 0x1, 0x1, 0xc0028d7e30, 0x0, ...)
        /usr/local/go/src/reflect/value.go:476 +0x8e7
reflect.Value.Call(0x8499e0, 0xc00001e070, 0x13, 0xc0028d7f28, 0x1, 0x1, 0xc00267a468, 0x27914776d46d2701, 0x2700000000000000)
        /usr/local/go/src/reflect/value.go:337 +0xb9
github.com/markus-wa/godispatch.callConsumerCode(0x8499e0, 0xc00001e070, 0x13, 0xc0028d7f28, 0x1, 0x1)
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:123 +0x8c
github.com/markus-wa/godispatch.(*Dispatcher).Dispatch(0xc0004ea1e0, 0x8c48a0, 0xc001ee5900)
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:109 +0x258
github.com/markus-wa/godispatch.(*Dispatcher).dispatchWithRecover(0xc0004ea1e0, 0x8c48a0, 0xc001ee5900)
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:195 +0x66
github.com/markus-wa/godispatch.(*Dispatcher).dispatchQueue(0xc0004ea1e0, 0xc0004ea2a0)
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:171 +0xf4
created by github.com/markus-wa/godispatch.(*Dispatcher).AddQueues
        /home/julien/go/pkg/mod/github.com/markus-wa/godispatch@v1.3.0/dispatch.go:157 +0x148
exit status 2

Expected behavior No deadlock

Additional context I have no idea if I'm using correctly Close or Cancel ?

markus-wa commented 3 years ago

Hi @Julien2313 - you can't use Close() inside event handlers, as it closes the event channels.

Cancel() should be fine but I can see that there's a bug when Cancel() is called multiple times before it's handled (it just sends a message to a channel). I'll have a look if Cancel() can be fixed.

What you can do for now is make sure you only send cancel once (e.g. by setting a flag the first time you call Cancel() and then not call it after that flag is set to true.

as for Close() - that should only be used after ParseToEnd() or ParseFrame() has returned and nothing is currently processing.

Julien2313 commented 3 years ago

Oooh ok ok, that's why ! Thank you for your fast answer :) Yeah, I put in the example Close() but I'm testing it with Cancel() (as your comment says)

I'll add a flag, thank you :)

markus-wa commented 3 years ago

Finally got around to this now.

@Julien2313 - would you be able to test my fix in this PR? https://github.com/markus-wa/demoinfocs-golang/pull/278

Julien2313 commented 3 years ago

It doesn't panic anymore, but it doesn't look like either to cancel the parsing :(

markus-wa commented 3 years ago

This was actually expected behaviour up until now (any parsed events would still be sent on). But after this change we don't need to do this anymore, so I've adjusted that as well.

Could you try again to see if it works with the latest commit on that PR?

Julien2313 commented 3 years ago

Working fine now ! :)

markus-wa commented 3 years ago

Thanks for the quick test @Julien2313 !

I've released v2.7.0 with the fix