Few potential race conditions when dynamically adding/removing conns and shutdown

lesismal / nbio

Pure Go 1000k+ connections solution, support tls/http1.x/websocket and basically compatible with net/http, with high-performance and low memory cost, non-blocking, event-driven, easy-to-use.

MIT License

2.17k stars 153 forks source link

Few potential race conditions when dynamically adding/removing conns and shutdown #402

Closed vrancurel closed 5 months ago

vrancurel commented 6 months ago

I discovered a few potential race conditions when running with -race option:

Dynamically adding and removing connections to the "epoll" engine due to non-locked access of connsUnix[], the problem is present I think in the other engines too.
At (g *Engine) Stop() when iterating over connsUnix (linked to above race)
Setting the shutdown flag at (p *poller) stop() which races with acceptorLoop test for shutdown.

You could see an attempt to fix them in this branch. Would it be possible to have a clean fix ? Thx

lesismal commented 6 months ago

Thanks for your feedback!

For *nix, each num of fd is unique at the same time, so, it's safe for us to visit the connsUnix by fd as index, extra Mutex would be just used for race fixing but not useful and waste more CPU cost.

We tried to disable the warning by go:norace but gave up finally.

lesismal commented 6 months ago

For the shutdown, it's a state check, extra Mutex just fix the race warning but useless for logic and waste more CPU cost.

lesismal commented 6 months ago

There are more race conditions, I don't want to avoid them by extra Mutex. If really need to do it, I think "go:norace" to ignore the warnings is better.

github-actions[bot] commented 5 months ago

This issue is stale because it has been open for 30 days with no activity.

github-actions[bot] commented 5 months ago

This issue was closed because it has been inactive for 14 days since being marked as stale.