lsalzman / enet

ENet reliable UDP networking library
MIT License
2.75k stars 670 forks source link

Deadlock in enet_protocol_send_outgoing_commands #154

Closed JohannesDeml closed 3 years ago

JohannesDeml commented 3 years ago

I'm using ENet with a wrapper with a .NET Core server application. I have one dedicated ENet thread receiving all events in a loop and one game loop thread prrocessing all incoming events and creating new outgoing messages. Everything works as expected, but when doing stresstests with 500 clients exchanging echo messages with the server I sometimes hit a deadlock with one thread getting stuck in the ENet method enet_protocol_send_outgoing_commands. I have this problem on an old Linux machine that is similar to the server I want to use. On my Windows machine, I haven't had any problems, even with 4K clients.

Since the deadlock occurs in enet, I'm posting my issue here. Here is a simplified version of what I'm doing:

ENet thread:

while (Running)
{
  bool polled = false;

  while (!polled)
  {
    if (host.CheckEvents(out netEvent) <= 0)
    {
      if (host.Service(config.MaxTimeout, out netEvent) <= 0)
        break;

      polled = true;
    }
    networkBuffer.WriteToInBuffer(netEvent);
  }
}

GameLoop Thread:

while (Running)
{
  while (incomingMessages.TryDequeue(out ENet.Event netEvent))
  {
    // process event
    networkBuffer.WriteToOutBuffer(netEvent.Peer, message);
    netEvent.Packet.Dispose();
  }

  var messagesToSend = networkBuffer.OutMessages.Count;
  for (int i = 0; i < messagesToSend; i++)
  {
    networkBuffer.OutMessages.TryDequeue(out var message);
    Packet packet = default(Packet);
    packet.Create(message.Data, message.Length, PacketFlags.None);
    message.Peer.Send(message.ChannelId, ref packet);
  }

  Thread.Sleep(5);
}

From what I found in the Mailing Lists is the following conversation: https://enet-discuss.cubik.narkive.com/U3tjOkMF/dangerous-about-function-enet-host-flush

However, I'm not calling flush and service at the same time. Is it a problem to have enet_host_service running while calling enet_peer_send from another thread? What's the best way to apply the results to enet?

nxrighthere commented 3 years ago

You have violated thread-safety in your logic. All operations that related to host and peers should remain thread-safe. In your case, host's service is called in one thread, while messages are concurrently enqueued for sending in another.

JohannesDeml commented 3 years ago

Thanks a lot for the response! That's what I somewhat feared. If I need to call all the sends in the ENet thread, I'm wondering if I can get host.Service(config.MaxTimeout, out netEvent) to return before MaxTimeout is reached and no event was received? Or do I need to use service nonblocking in order to enqueue the new packets as early as possible and then need to use Thread.Sleep() or similar?

JohannesDeml commented 3 years ago

After some more testing, I think the proposed idea to interrupt host.Service is more of a theoretical idea and is not really necessary in a real environment setup. The performance bottleneck I was encountering was caused by the game loop thread.

For reference, here is the changed loop that works great:

ENetLoop Thread:

while (Running)
{
  bool polled = false;

  while (!polled)
  {
    if (host.CheckEvents(out netEvent) <= 0)
    {
      if (host.Service(config.MaxTimeout, out netEvent) <= 0)
        break;

      polled = true;
    }

    networkBuffer.WriteToInBuffer(netEvent);
  }

  if(applyOutBuffer)
  {
    var messagesToSend = networkBuffer.OutMessages.Count;
    for (int i = 0; i < messagesToSend; i++)
    {
      networkBuffer.OutMessages.TryDequeue(out var message);
      Packet packet = default(Packet);
      packet.Create(message.Data, message.Length, PacketFlags.None);
      message.Peer.Send(message.ChannelId, ref packet);
    }

    host.Flush();
    applyOutBuffer = false;
  }
}

GameLoop Thread:

while (Running)
{
  while (incomingMessages.TryDequeue(out ENet.Event netEvent))
  {
    // process event
    networkBuffer.WriteToOutBuffer(netEvent.  , message);
    netEvent.Packet.Dispose();
  }

  networkThread.applyOutBuffer = true;
  Thread.Sleep(5); // Make a conscious decision of how to enforce the game loop time, this is just an example
}