C# multithread Socket - accès simultané possible

Question

je peux me tromper, mais il me semble que sur ce code une variable globale appelée _buffer est affecté à un nouvel objet sur le tas par plusieurs threads, donc si un fil essaie de lire les données qu'elle contient en fonction après avoir écrit dans une fonction précédente, mais entre-temps un autre thread a assinged cette _buffer variable à une autre objet sur le tas, je vais obtenir des données erronées. Est-ce que cela se passe vraiment ou est-ce que j'ai tort? Si c'est le cas, comment puis-je réparer?

public class SocketServer 
{ 
    Socket _serverSocket; 
    List<Socket> _clientSocket = new List<Socket>(); 
    byte[] _buffer; 

    public SocketServer() 
    { 
     _serverSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); 
    } 

    public void Bind(int Port) 
    { 
     Console.WriteLine("Setting up server..."); 
     _serverSocket.Bind(new IPEndPoint(IPAddress.Any, Port)); 
    } 

    public void Listen(int BackLog) 
    { 
     _serverSocket.Listen(BackLog); 
    } 

    public void Accept() 
    { 
     _serverSocket.BeginAccept(AcceptCallback, null); 
    } 

    private void AcceptCallback(IAsyncResult AR) 
    { 
     Socket socket = _serverSocket.EndAccept(AR); 
     _clientSocket.Add(socket); 
     Console.WriteLine("Client Connected"); 
     _buffer = new byte[1024]; 
     socket.BeginReceive(_buffer, 0, _buffer.Length, SocketFlags.None, ReceiveCallback, socket); 
     Accept(); 
    } 

    private void ReceiveCallback(IAsyncResult AR) 
    { 
     Socket socket = AR.AsyncState as Socket; 
     int bufferSize = socket.EndReceive(AR); 

     string text = Encoding.ASCII.GetString(_buffer, 0, bufferSize); 
     Console.WriteLine("Text Received: {0}", text); 

     string response = string.Empty; 

     if (text.ToLower() != "get time") 
      response = $"\"{text}\" is a Invalid Request"; 
     else 
      response = DateTime.Now.ToLongTimeString(); 

     byte[] data = Encoding.ASCII.GetBytes(response); 
     socket.BeginSend(data, 0, data.Length, SocketFlags.None, SendCallback, socket); 

     _buffer = new byte[1024]; 
     socket.BeginReceive(_buffer, 0, _buffer.Length, SocketFlags.None, ReceiveCallback, socket); 
    } 

    private void SendCallback(IAsyncResult AR) 
    { 
     (AR.AsyncState as Socket).EndSend(AR); 
    } 
} 

Answer 1

Il existe un data race lorsque plusieurs threads sont impliqués, et au moins l'un d'entre eux est un writer.

Socket est thread-safe, mais SocketServern'est pas. Vous écrivez à _buffer immédiatement avant de l'utiliser. C'est très certainement une course de données dans un scénario multithread. Vous avez besoin d'un mécanisme de verrouillage autour de chaque accès à l'état partagé.

Il ne sert à rien d'utiliser un champ pour _buffer si vous l'écrasez immédiatement avant de le transmettre. Si vous avez besoin d'un tampon pour travailler avec, attribuez-le une fois au moment de l'initialisation. Pour éviter de changer trop, vous pouvez la mettre en œuvre comme ceci:

class SocketServer 
{ 
    class Transaction 
    { 
     public readonly byte[] Data; 
     public readonly Socket Socket; 

     public Transaction(byte[] data, Socket socket) 
     { 
      Data = data; 
      Socket = socket; 
     } 
    } 

    private readonly object _syncObj = new object(); 
    private readonly List<Transaction> _received = new List<Transaction>(); 
    //... 

    //... 
    private void AcceptCallback(IAsyncResult AR) 
    { 
     //... 
     byte[] buffer = new byte[1024]; 
     socket.BeginReceive(
      buffer, 0, buffer.Length, SocketFlags.None, 
      ReceiveCallback, new Transaction(buffer, socket)); 
     //... 
    } 
    private void ReceiveCallback(IAsyncResult AR) 
    { 
     Transaction trans = (Transaction)AR.AsyncState; 
     Socket socket = trans.Socket; 
     int bufferSize = socket.EndReceive(AR); 
     lock (_syncObj) { 
      _received.Add(trans); 
     } 
     //... 
     byte[] buffer = new byte[1024]; 
     socket.BeginReceive(
      buffer, 0, buffer.Length, SocketFlags.None, 
      ReceiveCallback, new Transaction(buffer, socket)); 
    } 
    //... 

    // Call this to get all the received data. 
    // This will block ReceiveCallback until it completes. 
    public byte[] GetReceivedData() 
    { 
     int totalSize = 0; 
     lock (_syncObj) { 
      for (int i = 0; i < _received.Length; i++) { 
       totalSize += _received[i].Data.Length; 
      } 

      byte[] totalData = new byte[totalSize]; 
      int offset = 0; 
      for (int i = 0; i < _received.Length; i++) { 
       byte[] blockData = _received[i].Data; 
       Buffer.BlockCopy(blockData, 0, totalData, offset, blockData.Length); 
       offset += blockData.Length; 
      } 
      _received.Clear(); 
      return totalData; 
     } 
    } 
} 

vous pourriez aussi créer une implémentation thread-safe de IList<ArraySegment<byte>> et utiliser le overloads approprié, mais qui est hors de la portée de cette réponse.

Sur une note non liée, vos conventions de dénomination sont incohérentes. Vous utilisez le cas underscore de chameau pour les champs, un mélange de majuscules & cas pour les paramètres pascals, et cas de chameau pour les variables locales. Utilisez toutes les conventions que vous désirez, mais s'il vous plaît soyez cohérent. Je suggère de suivre le general guidelines.

Answer 2

_buffer n'est pas adapté aux threads. J'utiliserais une collection concurrente comme ConcurrentBag au lieu d'un tableau d'octets plan. Cela garantira la sécurité du fil pour vous. Si vous voulez garder _buffer comme un tableau, vous devez utiliser des verrous appropriés (par exemple en utilisant le mot-clé de verrouillage) pour vous assurer que plusieurs threads en même temps ne pas essayer d'accéder _buffer. En savoir plus sur ConcurrentBag: https://msdn.microsoft.com/en-us/library/dd381779(v=vs.110).aspx