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

Queue locking strategy error in Client Socket #19

Open
magikworx opened this issue May 27, 2018 · 1 comment
Open

Queue locking strategy error in Client Socket #19

magikworx opened this issue May 27, 2018 · 1 comment

Comments

@magikworx
Copy link

The queue locking strategy is incorrect for the SocketContext.cs(68-70) call to
if (datagramQueue.Count > 0) { packet = datagramQueue.Dequeue();
In a highly concurrent environment, the queue isn't being locked for the entire transaction and a thread can pull an item off the queue making an empty queue when the datagram dequeue is called and causing a System.InvalidOperationException.
Maybe you should try to convert this to a CAS style operation or move the storage to a tested concurrent data structure.

@magikworx
Copy link
Author

magikworx commented May 27, 2018

I changed your DatagramQueue to read as follows

public bool TryDequeue(out Datagram datagram)
{
  lock (queue_mutex){
    if (datagramQueue.Count > 0)
    {
      datagram = datagramQueue.Dequeue();
      return true;
    }  
  }
  datagram = new Datagram();
  return false;
}

The enqueue call in the Pump() should also be updated to use the Enqueue method inside the class and not directly on the queue.

The bonus of TryDequeue is that it simplifies the calls in the SocketContext as follows

public bool Read(out Datagram packet)
{
  return datagramQueue.TryDequeue(out packet);
}

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