antchfx / xmlquery

xmlquery is Golang XPath package for XML query.
https://github.com/antchfx/xpath
MIT License
444 stars 89 forks source link

Race condition in getQuery #26

Closed sgoldenb closed 4 years ago

sgoldenb commented 4 years ago

Hi, i have a project in which multiple go routines parse xml files in parallel. Today i got the following crash:

fatal error: concurrent map read and map write goroutine 509 [running]: runtime.throw(0xaf97d6, 0x21) /usr/lib/go-1.13/src/runtime/panic.go:774 +0x72 fp=0xc004fb9678 sp=0xc004fb9648 pc=0x446712 runtime.mapaccess2(0xa20ec0, 0xc000299a10, 0xc004fb96e8, 0xc0002afef8, 0xc0002f68d8) /usr/lib/go-1.13/src/runtime/map.go:470 +0x278 fp=0xc004fb96c0 sp=0xc004fb9678 pc=0x4255c8 github.com/golang/groupcache/lru.(*Cache).Get(0xc003a7b640, 0x9fa140, 0xc004fb9740, 0xc0002f68d8, 0xc0034b6200, 0x390000c004774720) /root/go/pkg/mod/github.com/golang/groupcache@v0.0.0-20191027212112-611e8accdfc9/lru/lru.go:78 +0x68 fp=0xc004fb9708 sp=0xc004fb96c0 pc=0x8d88a8 github.com/antchfx/xmlquery.getQuery(0xc0000248e0, 0x20, 0x11d3ef0, 0x0, 0xc0002afee0) /root/go/pkg/mod/github.com/antchfx/xmlquery@v1.2.1/cache.go:30 +0xbd fp=0xc004fb9760 sp=0xc004fb9708 pc=0x91359d github.com/antchfx/xmlquery.Query(0xc00017cc00, 0xc0000248e0, 0x20, 0xc0034b6200, 0x0, 0xc0000248a0) /root/go/pkg/mod/github.com/antchfx/xmlquery@v1.2.1/query.go:103 +0x39 fp=0xc004fb9798 sp=0xc004fb9760 pc=0x9162e9 github.com/antchfx/xmlquery.FindOne(...) /root/go/pkg/mod/github.com/antchfx/xmlquery@v1.2.1/query.go:83

In findOne there is a package variable for the not thread save groupace/lru and since i use xmlqery concurrently the whole thing can crash here.

It is possible to disable the cache but i am not sure how much this would impact performance.

I guess that adding a mutex here would not impact performance as much as disabling the cache completely.

Should i write a pr for this?

Best regards Sebastian

zhengchun commented 4 years ago

It is an open project, You can submit a PR anytime.

zhengchun commented 4 years ago

htmlquery package have the same bug: https://github.com/antchfx/htmlquery/issues/19

vtolstov commented 4 years ago

I'm take this, as i already have fix internally.

zhengchun commented 4 years ago

Closed. It fixed in Release 1.2.2.