Skip to content

Commit b0d84be

Browse files
markus-wamarkus-wa
authored
bitread: upgrade gobitread for better handling of streams + fix resource leak (#305)
(hopefully) fixes issue where not enough bytes to fill the buffer are read, but the stream is not yet at the end
1 parent 4853b1f commit b0d84be

File tree

9 files changed

+48
-30
lines changed

9 files changed

+48
-30
lines changed

‎go.mod

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ require (
66
github.com/golang/geo v0.0.0-20210211234256-740aa86cb551
77
github.com/llgcode/draw2d v0.0.0-20200930101115-bfaf5d914d1e
88
github.com/markus-wa/go-unassert v0.1.2
9-
github.com/markus-wa/gobitread v0.2.2
9+
github.com/markus-wa/gobitread v0.2.3
1010
github.com/markus-wa/godispatch v1.4.1
1111
github.com/markus-wa/quickhull-go/v2 v2.1.0
12+
github.com/pkg/errors v0.9.1
1213
github.com/stretchr/testify v1.6.1
1314
)
1415

‎go.sum

+6-18
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
22
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
33
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
4-
github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
54
github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
6-
github.com/go-gl/gl v0.0.0-20180407155706-68e253793080 h1:pNxZva3052YM+z2p1aP08FgaTE2NzrRJZ5BHJCmKLzE=
75
github.com/go-gl/gl v0.0.0-20180407155706-68e253793080/go.mod h1:482civXOzJJCPzJ4ZOX/pwvXBWSnzD4OKMdH4ClKGbk=
8-
github.com/go-gl/glfw v0.0.0-20180426074136-46a8d530c326 h1:QqWaXlVeUGwSH7hO8giZP2Y06Qjl1LWR+FWC22YQsU8=
96
github.com/go-gl/glfw v0.0.0-20180426074136-46a8d530c326/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
107
github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
118
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
@@ -14,11 +11,8 @@ github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGw
1411
github.com/golang/geo v0.0.0-20180826223333-635502111454/go.mod h1:vgWZ7cu0fq0KY3PpEHsocXOWJpRtkcbKemU4IUw0M60=
1512
github.com/golang/geo v0.0.0-20210211234256-740aa86cb551 h1:gtexQ/VGyN+VVFRXSFiguSNcXmS6rkKT+X7FdIrTtfo=
1613
github.com/golang/geo v0.0.0-20210211234256-740aa86cb551/go.mod h1:QZ0nwyI2jOfgRAoBvP+ab5aRr7c9x7lhGEJrKvBwjWI=
17-
github.com/jung-kurt/gofpdf v1.0.0 h1:EroSdlP9BOoL5ssLYf3uLJXhCQMMM2fFxCJDKA3RhnA=
1814
github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
19-
github.com/kisielk/errcheck v1.5.0 h1:e8esj/e4R+SAOwFwN+n3zr0nYeCyeweozKfO23MvHzY=
2015
github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
21-
github.com/kisielk/gotool v1.0.0 h1:AV2c/EiW3KqPNT9ZKl07ehoAGi4C5/01Cfbblndcapg=
2216
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
2317
github.com/llgcode/draw2d v0.0.0-20200930101115-bfaf5d914d1e h1:YRRazju3DMGuZTSWEj0nE2SCRcK3DW/qdHQ4UQx7sgs=
2418
github.com/llgcode/draw2d v0.0.0-20200930101115-bfaf5d914d1e/go.mod h1:mVa0dA29Db2S4LVqDYLlsePDzRJLDfdhVZiI15uY0FA=
@@ -28,12 +22,16 @@ github.com/markus-wa/go-heatmap v1.0.0 h1:UT3+9Re6Fr6h4Qs7y0QmVjlvbu+92E9tnuqKEh
2822
github.com/markus-wa/go-heatmap v1.0.0/go.mod h1:mUE6YBiMclq9MYXCb8fLbiWvyZtGglLrfMJ+LSHrlPA=
2923
github.com/markus-wa/go-unassert v0.1.2 h1:uXWlMDa8JVtc4RgNq4XJIjyRejv9MOVuy/E0VECPxxo=
3024
github.com/markus-wa/go-unassert v0.1.2/go.mod h1:XEvrxR+trvZeMDfXcZPvzqGo6eumEtdk5VjNRuvvzxQ=
31-
github.com/markus-wa/gobitread v0.2.2 h1:4Z4oJ8Bf1XnOy6JZ2/9AdFKVAoxdq7awRjrb+j2BeSQ=
32-
github.com/markus-wa/gobitread v0.2.2/go.mod h1:PcWXMH4gx7o2CKslbkFkLyJB/aHW7JVRG3MRZe3PINg=
25+
github.com/markus-wa/gobitread v0.2.3-0.20210710124713-58959e776f8b h1:iYFgASuNE8XtbTxjWq1AoO2uYJ4knR1AgsGfrW6byz4=
26+
github.com/markus-wa/gobitread v0.2.3-0.20210710124713-58959e776f8b/go.mod h1:PcWXMH4gx7o2CKslbkFkLyJB/aHW7JVRG3MRZe3PINg=
27+
github.com/markus-wa/gobitread v0.2.3 h1:COx7dtYQ7Q+77hgUmD+O4MvOcqG7y17RP3Z7BbjRvPs=
28+
github.com/markus-wa/gobitread v0.2.3/go.mod h1:PcWXMH4gx7o2CKslbkFkLyJB/aHW7JVRG3MRZe3PINg=
3329
github.com/markus-wa/godispatch v1.4.1 h1:Cdff5x33ShuX3sDmUbYWejk7tOuoHErFYMhUc2h7sLc=
3430
github.com/markus-wa/godispatch v1.4.1/go.mod h1:tk8L0yzLO4oAcFwM2sABMge0HRDJMdE8E7xm4gK/+xM=
3531
github.com/markus-wa/quickhull-go/v2 v2.1.0 h1:DA2pzEzH0k5CEnlUsouRqNdD+jzNFb4DBhrX4Hpa5So=
3632
github.com/markus-wa/quickhull-go/v2 v2.1.0/go.mod h1:bOlBUpIzGSMMhHX0f9N8CQs0VZD4nnPeta0OocH7m4o=
33+
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
34+
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
3735
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
3836
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
3937
github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=
@@ -42,46 +40,36 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P
4240
github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
4341
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
4442
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
45-
github.com/yuin/goldmark v1.2.1 h1:ruQGxdhGHe7FWOJPT0mKs5+pD2Xs1Bm/kdGlHO04FmM=
4643
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
4744
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
4845
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
49-
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 h1:psW17arqaxU48Z5kZ0CQnkZWQJsqcURM6tKiBApRjXI=
5046
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
5147
golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81 h1:00VmoueYNlNz/aHIilyyQz/MHSqGoWJzpFv/HW8xpzI=
5248
golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs=
5349
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
54-
golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
5550
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
5651
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
5752
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
5853
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
59-
golang.org/x/net v0.0.0-20201021035429-f5854403a974 h1:IX6qOQeG5uLjB/hjjwjedwfjND0hgjPMMyO1RoIXQNI=
6054
golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
6155
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
6256
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
63-
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9 h1:SQFwaSi55rU7vdNs9Yr0Z324VNlrF+0wMqRXT4St8ck=
6457
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
6558
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
6659
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
67-
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f h1:+Nyd8tzPX9R7BWHguqsrbFdRx3WQ/1ib8I44HXV5yTA=
6860
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
6961
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
70-
golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k=
7162
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
7263
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
7364
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
7465
golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
75-
golang.org/x/tools v0.0.0-20210106214847-113979e3529a h1:CB3a9Nez8M13wwlr/E2YtwoU+qYHKfC+JrDa45RXXoQ=
7666
golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
7767
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
7868
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
7969
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
80-
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
8170
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
8271
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
8372
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
84-
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
8573
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
8674
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
8775
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

‎internal/bitread/bitread.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"sync"
1010

1111
bitread "github.com/markus-wa/gobitread"
12+
"github.com/pkg/errors"
1213
)
1314

1415
const (
@@ -96,8 +97,11 @@ var bitReaderPool = sync.Pool{
9697

9798
// Pool puts the BitReader into a pool for future use.
9899
// Pooling BitReaders improves performance by minimizing the amount newly allocated readers.
99-
func (r *BitReader) Pool() {
100-
r.Close()
100+
func (r *BitReader) Pool() error {
101+
err := r.Close()
102+
if err != nil {
103+
return errors.Wrap(err, "failed to close BitReader before pooling")
104+
}
101105

102106
if len(*r.buffer) == smallBuffer {
103107
smallBufferPool.Put(r.buffer)
@@ -106,6 +110,8 @@ func (r *BitReader) Pool() {
106110
r.buffer = nil
107111

108112
bitReaderPool.Put(r)
113+
114+
return nil
109115
}
110116

111117
func newBitReader(underlying io.Reader, buffer *[]byte) *BitReader {

‎pkg/demoinfocs/fake/parser.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,6 @@ func (p *Parser) Cancel() {
229229

230230
// Close is a mock-implementation of Parser.Close().
231231
// NOP implementation.
232-
func (p *Parser) Close() {
233-
p.Called()
232+
func (p *Parser) Close() error {
233+
return p.Called().Error(0)
234234
}

‎pkg/demoinfocs/net_messages.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,12 @@ func (p *parser) handlePacketEntities(pe *msg.CSVCMsg_PacketEntities) {
4747
}
4848
}
4949

50-
r.Pool()
50+
err := r.Pool()
51+
if err != nil {
52+
p.eventDispatcher.Dispatch(events.ParserWarn{
53+
Message: err.Error(),
54+
})
55+
}
5156
}
5257

5358
func (p *parser) handleSetConVar(setConVar *msg.CNETMsg_SetConVar) {

‎pkg/demoinfocs/parser.go

+12-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/gogo/protobuf/proto"
1111
"github.com/golang/geo/r3"
1212
dp "github.com/markus-wa/godispatch"
13+
"github.com/pkg/errors"
1314

1415
bit "github.com/markus-wa/demoinfocs-golang/v2/internal/bitread"
1516
common "github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs/common"
@@ -236,8 +237,18 @@ func (p *parser) UnregisterNetMessageHandler(identifier dp.HandlerIdentifier) {
236237

237238
// Close closes any open resources used by the Parser (go routines, file handles).
238239
// This must be called before discarding the Parser to avoid memory leaks.
239-
func (p *parser) Close() {
240+
// Returns an error if closing of underlying resources fails.
241+
func (p *parser) Close() error {
240242
p.msgDispatcher.RemoveAllQueues()
243+
244+
if p.bitReader != nil {
245+
err := p.bitReader.Close()
246+
if err != nil {
247+
return errors.Wrap(err, "failed to close BitReader")
248+
}
249+
}
250+
251+
return nil
241252
}
242253

243254
func (p *parser) error() error {

‎pkg/demoinfocs/parser_interface.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ type Parser interface {
101101
UnregisterNetMessageHandler(identifier dp.HandlerIdentifier)
102102
// Close closes any open resources used by the Parser (go routines, file handles).
103103
// This must be called before discarding the Parser to avoid memory leaks.
104-
Close()
104+
// Returns an error if closing of underlying resources fails.
105+
Close() error
105106
// ParseHeader attempts to parse the header of the demo and returns it.
106107
// If not done manually this will be called by Parser.ParseNextFrame() or Parser.ParseToEnd().
107108
//

‎pkg/demoinfocs/parser_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,8 @@ func TestParser_Close(t *testing.T) {
128128
called = true
129129
})
130130

131-
p.Close()
131+
err := p.Close()
132+
assert.NoError(t, err)
132133

133134
q <- "this should not trigger the handler"
134135

‎pkg/demoinfocs/parsing.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package demoinfocs
22

33
import (
4-
"errors"
54
"fmt"
65
"io"
76
"math"
@@ -11,6 +10,7 @@ import (
1110
"github.com/gogo/protobuf/proto"
1211
"github.com/markus-wa/go-unassert"
1312
dispatch "github.com/markus-wa/godispatch"
13+
"github.com/pkg/errors"
1414

1515
common "github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs/common"
1616
events "github.com/markus-wa/demoinfocs-golang/v2/pkg/demoinfocs/events"
@@ -340,10 +340,15 @@ func (p *parser) parsePacket() {
340340
p.bitReader.ReadBytesInto(b, size)
341341

342342
m := msgCreator()
343-
if proto.Unmarshal(*b, m) != nil {
343+
344+
err := proto.Unmarshal(*b, m)
345+
if err != nil {
344346
// TODO: Don't crash here, happens with demos that work in gotv
345-
panic(fmt.Sprintf("Failed to unmarshal cmd %d", cmd))
347+
p.setError(errors.Wrapf(err, "failed to unmarshal cmd %d", cmd))
348+
349+
return
346350
}
351+
347352
p.msgQueue <- m
348353

349354
// Reset length to 0 and pool

0 commit comments

Comments
 (0)