Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing Thread Synchronization #12

Open
mfya opened this issue Apr 14, 2017 · 0 comments
Open

Missing Thread Synchronization #12

mfya opened this issue Apr 14, 2017 · 0 comments

Comments

@mfya
Copy link

mfya commented Apr 14, 2017

First let me tell that this is very nice project and a good demonstration of several open source libraries. However, I noticed some small but serious flaws, which cause instability and need to be fixed before it can be used reliably.

The Boost.Asio io_service.run() is called in a worker thread, which means all async signal handlers are also executed on that worker thread. Because of this all async signal handlers execute in parallel with the main thread but the code currently lacks any thread synchronization.

One easy to spot race condition exists for the Client::inMessages deque, which is accessed by Client::read() from the main thread and by the async handler in Client::receive() in a worker thread. There are also races caused by other async signal handlers in both the client and the server code.

A simple fix should be to call io_service::poll() from the main loop instead of calling io_service::run() in a thread, so all async signal handlers are also executed on the main thread. Or of course adding a scoped lock inside all methods.

Another small thing I noticed, which is only slightly related, is that the example_game main loop unnecessarily maxes out one CPU core because it never sleeps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant