OpenDiablo2 icon indicating copy to clipboard operation
OpenDiablo2 copied to clipboard

Server host's client sometimes crashes when another client connects

Open ianling opened this issue 5 years ago • 1 comments

Describe the bug The host's game can crash when another client connects due to the renderer reading from the map of entities at the same time that the new player is added to that map. In other words, two goroutines are accessing one map at the same time.

[Game Server][INFO] Accepting connection: 127.0.0.1:38860

[Game Server][INFO] Client connected with an id of cf9b10b7-654e-48d5-aeed-8c1d69733898
fatal error: concurrent map iteration and map write
[... stack trace...]

The two functions trying to read and write the map at the same time are:

  • d2core/d2map/d2maprenderer/renderer.go:244 (for _, mapEntity := range mr.mapEngine.Entities() {)
  • d2networking/d2client/game_client.go:227 (g.MapEngine.AddEntity(newPlayer))

There is some other code that writes to this map as well, so whoever picks this up should consider implementing a fix that handles all cases, not just this one.

To Reproduce Steps to reproduce the behavior:

  1. Start two clients.
  2. On one client, host a multiplayer game
  3. Join the game on the other client
  4. The host's game may crash (doesn't happen every time, race condition)

Expected behavior The second client joins the host's game successfully, no one crashes

Desktop (please complete the following information):

  • OS: Xubuntu 20.04 x86_64

Not sure what the best solution would be here. Maybe reading the list of entities should happen from a copy of the map and not from a pointer?

ianling avatar Dec 26 '20 21:12 ianling

If data is being written or read to via a thread, we've gotta make sure those actions are thread safe. Not just for this, but for anything in the codebase.

essial avatar Dec 27 '20 00:12 essial