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

Server crashes if cPlayer is deleted during the adding procedure. #1969

Closed
worktycho opened this issue May 8, 2015 · 4 comments · Fixed by #2136
Closed

Server crashes if cPlayer is deleted during the adding procedure. #1969

worktycho opened this issue May 8, 2015 · 4 comments · Fixed by #2136

Comments

@worktycho
Copy link
Member

There is a race condition in the server that for a sufficiently loaded server, if the connection is closed during the player spawn process the server will crash as cWorld::HasEntity returns true for Entitys in the queue, but cWorld::Player does not remove it from. This causes the server to crash due to an ASSERT in cEntity.

I achieved this situation with a c++ program that simulates the bare minimum of a client to login, and attempts to run a login against the server every 10 ms.

@sphinxc0re
Copy link
Contributor

Double-free?

@worktycho
Copy link
Member Author

No, an assert failure, and potential use after free.

@worktycho
Copy link
Member Author

If anyone wants to reproduce this, or try and find other race conditions, I can post the program.

@tigerw
Copy link
Member

tigerw commented May 12, 2015

Something else that may be related:

If the world thread is blocked somewhere (i.e. in cMonster::Tick) for long enough, the client quits because of a timeout. cClientHandle's m_Protocol is destroyed, but then a message tries to be sent to the quit player, but since m_Protocol is null, there is a crash.

worktycho added a commit that referenced this issue May 25, 2015
when socket is closed whilst the client is being added to the world
Fixes #1969
worktycho added a commit that referenced this issue May 26, 2015
when socket is closed whilst the client is being added to the world
Fixes #1969
tigerw added a commit that referenced this issue May 28, 2015
* Potentially addresses my comment in #1969
* Probably fixes #2145
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

Successfully merging a pull request may close this issue.

3 participants