search

simonvetter / modbus

Go modbus stack (client and server)
MIT License
280 stars 89 forks source link

fix race conditions #16

Closed thibaudroy closed 2 years ago

thibaudroy commented 2 years ago

First, thanks for bringing this very neat Golang Modbus stack :pray:

I faced race conditions while writing down tests where I was successively opening/closing the server and reconnecting TCP clients

Reason is: acceptTCPClients() which is running in a separate goroutine might concurrently access to some attributes.

When I tried to reproduce this error with

go test -timeout 30s  github.com/simonvetter/modbus -race`

I spot another race condition in unit tests.

I'll propose a PR, your opinion we'll be appreciated !

thibaudroy commented 2 years ago

@simonvetter looks like I'm not allowed to create a branch/PR. I'm ready to push on my side (commit adding getters holding read locks), let me know if you're interested

simonvetter commented 2 years ago

Hey man, thanks for reporting it! Glad to see you around hacking on open source projects :-) Are those races only present in tests, or is it in the server itself?

One way of proceeding would be to fork my repo, push to your fork, then create a pull request here. Would that work for you?

thibaudroy commented 2 years ago

Hey man, thanks for reporting it! Glad to see you around hacking on open source projects :-) Are those races only present in tests, or is it in the server itself?

To my understanding, the race against access to .started occured at server level in my test suite, while for .tcpClients it was at unit test level

One way of proceeding would be to fork my repo, push to your fork, then create a pull request here. Would that work for you?

Sure ! Done :)

simonvetter commented 2 years ago

Sorry for the late reply, I've been fairly busy lately.

Does https://github.com/simonvetter/modbus/tree/simon/fix-test-races solve your issues? If yes, I'll merge and do a point release.

thibaudroy commented 2 years ago

It does ! Thanks @simonvetter. Have a good day !

simonvetter commented 2 years ago

nice! v1.5.1 pushed. Thanks again!