search

0xrawsec / golang-evtx

GNU General Public License v3.0
158 stars 27 forks source link

attempt at fixing data race in #25 #26

Closed steve-offutt closed 4 years ago

steve-offutt commented 4 years ago

go build -race tools/evtxdump/... && ./evtxdump -s $EVTX_FILE produces a race warning. It was observed that the data race could reside here: https://github.com/0xrawsec/golang-evtx/blob/fb1efdb926d27bb62f9b82f532fdd6a66fa14741/evtx/utils.go#L80-L83 This is most likely due to the fact that multiple goroutines are attempting to alter the global array lastParsedElements. After commenting out the function body of UpdateLastElements and rerunning the same commands mentioned above, there is no longer a race condition warning.

I'm not aware of the purpose of lastParsedElements. However, the comment for the variable mentions that it is for debug purposes so I assume that the impact of commenting out the code in UpdateLastElements to be small.

steve-offutt commented 4 years ago

@qjerome Adding a RWMutex to the [4]Element global used for debugging fixes the race condition observed when building with -race. This fix is useful for people who use this package in a CI/CD pipeline that require the -race flag be set in their build to detect race conditions. I know that this variable is only used for debugging. However, this race breaks CI/CD.

qjerome commented 4 years ago

I thought about this solution however it breaks the whole purpose of parallelization. Having a shared resource with a mutex across thread will cause threads to wait for that resource and will decrease performances. I will try to come up with another fix.