core: fix deadlock caused by multiple calls to Parser.Cancel() (#276)

- upgrade to [email protected] so we can use Dispatcher.UnregisterAllHandlers()

Author: markus-wa

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/llgcode/draw2d v0.0.0-20200930101115-bfaf5d914d1e
 	github.com/markus-wa/go-unassert v0.1.2
 	github.com/markus-wa/gobitread v0.2.2
-	github.com/markus-wa/godispatch v1.3.0
+	github.com/markus-wa/godispatch v1.4.1
 	github.com/markus-wa/quickhull-go/v2 v2.1.0
 	github.com/stretchr/testify v1.6.1
 	golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b // indirect

go.sum

@@ -43,6 +43,10 @@ github.com/markus-wa/godispatch v1.2.1 h1:Bj6blK4xJ/72pDi0rprVYluOGW9CV55FUDFPAp
 github.com/markus-wa/godispatch v1.2.1/go.mod h1:GNzV7xdnZ9+VBi0z+hma9oUQrJmtqRrqyAuGKTTTcKY=
 github.com/markus-wa/godispatch v1.3.0 h1:eHT5Xm8ZEwilj9b/Tu7gyMfmtSau+yC0qpM7NRR+CQk=
 github.com/markus-wa/godispatch v1.3.0/go.mod h1:GNzV7xdnZ9+VBi0z+hma9oUQrJmtqRrqyAuGKTTTcKY=
+github.com/markus-wa/godispatch v1.4.0 h1:cSRg6/jvLjpi2616rJAbgXMfl8gV+6RfEAUe+8qVfYE=
+github.com/markus-wa/godispatch v1.4.0/go.mod h1:oz4Bh/xA+T/tMR5jXShPQ3fhkL5ba6VsVg9hLZzpEbw=
+github.com/markus-wa/godispatch v1.4.1 h1:Cdff5x33ShuX3sDmUbYWejk7tOuoHErFYMhUc2h7sLc=
+github.com/markus-wa/godispatch v1.4.1/go.mod h1:tk8L0yzLO4oAcFwM2sABMge0HRDJMdE8E7xm4gK/+xM=
 github.com/markus-wa/quickhull-go/v2 v2.1.0 h1:DA2pzEzH0k5CEnlUsouRqNdD+jzNFb4DBhrX4Hpa5So=
 github.com/markus-wa/quickhull-go/v2 v2.1.0/go.mod h1:bOlBUpIzGSMMhHX0f9N8CQs0VZD4nnPeta0OocH7m4o=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=

pkg/demoinfocs/parser.go

@@ -59,7 +59,6 @@ type parser struct {
 	header                       *common.DemoHeader // Pointer so we can check for nil
 	gameState                    *gameState
 	demoInfoProvider             demoInfoProvider // Provides demo infos to other packages that the core package depends on
-	cancelled                    bool             // Indicates if Parser.Cancel() has been called
 	err                          error            // Contains a error that occurred during parsing if any
 	errLock                      sync.Mutex       // Used to sync up error mutations between parsing & handling go-routines
 
@@ -250,12 +249,20 @@ func (p *parser) error() error {
 }
 
 func (p *parser) setError(err error) {
-	if err == nil || p.err != nil {
+	if err == nil {
 		return
 	}
 
 	p.errLock.Lock()
+
+	if p.err != nil {
+		p.errLock.Unlock()
+
+		return
+	}
+
 	p.err = err
+
 	p.errLock.Unlock()
 }
 

pkg/demoinfocs/parser_interface.go

@@ -112,8 +112,8 @@ type Parser interface {
 	//
 	// See also: ParseNextFrame() for other possible errors.
 	ParseToEnd() (err error)
-	// Cancel aborts ParseToEnd().
-	// All information that was already read up to this point may still be used (and new events may still be sent out).
+	// Cancel aborts ParseToEnd() and drains the internal event queues.
+	// No further events will be sent to event or message handlers after this.
 	Cancel()
 	/*
 	   ParseNextFrame attempts to parse the next frame / demo-tick (not ingame tick).

pkg/demoinfocs/parsing.go

@@ -111,14 +111,8 @@ func (p *parser) ParseToEnd() (err error) {
 	}
 
 	for {
-		select {
-		case <-p.cancelChan:
-			return ErrCancelled
-
-		default:
-			if !p.parseFrame() {
-				return p.error()
-			}
+		if !p.parseFrame() {
+			return p.error()
 		}
 
 		if err = p.error(); err != nil {
@@ -145,10 +139,12 @@ func recoverFromUnexpectedEOF(r interface{}) error {
 	}
 }
 
-// Cancel aborts ParseToEnd().
-// All information that was already read up to this point may still be used (and new events may still be sent out).
+// Cancel aborts ParseToEnd() and drains the internal event queues.
+// No further events will be sent to event or message handlers after this.
 func (p *parser) Cancel() {
-	p.cancelChan <- struct{}{}
+	p.setError(ErrCancelled)
+	p.eventDispatcher.UnregisterAllHandlers()
+	p.msgDispatcher.UnregisterAllHandlers()
 }
 
 /*