C # - запутался на замке

Question

Для моего сетевого проекта мне нужно заблокировать некоторые коды, чтобы предотвратить одновременный доступ. Это мой код

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Utility;
using DataBaseConnection;
using System.Net.Sockets;
using System.Data;
using System.IO;

namespace SunHavenClasses
{
    public delegate void CtatReceiveDelegate(string message);
    public class ServerHandlerClass
    {
        public event CtatReceiveDelegate OnChatDataReceive;
        private Settings settings;
        private DBCon con;
        private Utility.Network.Server server;
        private Dictionary<string, Socket> UsersOnline;
        private Dictionary<string, int> unAuthenticatedIps;
        private string pass = "logisoftlogicielbarundipankar";
        public ServerHandlerClass(Settings s)
        {
            settings = s;
            con = s.GetConnection();
            server = new Utility.Network.Server(7777);
            server.ClientConnectEventArise += OnUserConnect;//.OnClientConnect(OnUserConnect);
            server.ClientDataReceiveEventArise += OnUsersDataReceive;
            server.ClientDataSendEventArise += OnDataSendToUser;
            server.ClientDisconnectEventArise += OnUserDisconnect;
            server.OnBlockUser += OnUserBlocked;

            UsersOnline = new Dictionary<string, Socket>();
            unAuthenticatedIps = new Dictionary<string, int>();
        }

        private void OnUserConnect(Utility.Network.ServerEventArguments e)
        {
            Stream data = Utility.Serializing.Serialize(settings);
            data = ZipNEncrypt.Zip(new string[] { "settings" }, new Stream[] { data }, pass);
            server.Send(data, e.ClientSocket);
            //MessageBox.Show(e.ClientSocket.RemoteEndPoint.ToString() + " is Connected!!");
        }
        private void OnUsersDataReceive(Utility.Network.ServerEventArguments e)
        {
            Dictionary<string, System.IO.Stream> data = ZipNEncrypt.Unzip(e.Data, pass);
            User user;
            try
            {
                user = (User)Serializing.Deserialize(data["user"]);
                if (!UsersOnline.ContainsKey(user.GetUserId()))
                {
                    server.BlockIp(e.ClientSocket);
                    return;
                }
                data.Remove("user");
            }
            catch (Exception)
            {
                bool passed = true;
                foreach (string key in data.Keys)
                {
                    if (key.Equals("LoggedIn")) break;
                    string[] str = key.Split('_');
                    if (str[0].Equals("GetData"))
                    {
                        string strr = (string)Serializing.Deserialize(data[key]);
                        if (strr.Contains("Users"))
                        {
                            string ip = e.ClientSocket.RemoteEndPoint.ToString().Split(':')[0];
                            /*CHANGE 1.2.10 00:14*/
                            lock (unAuthenticatedIps)
                            {
                                if (!unAuthenticatedIps.ContainsKey(ip))
                                {
                                    unAuthenticatedIps.Add(ip, 1);
                                }
                                else unAuthenticatedIps[ip] += 1;
                                if (unAuthenticatedIps[ip] >= 11) passed = false;
                            }
                            /*CHANGE 1.2.10 00:14*/
                            break;
                        }
                        else passed = false;//server.AddBlockedIp(ip);
                    }
                    else passed = false;
                }
                if (!passed)
                {
                    server.BlockIp(e.ClientSocket);
                }
            }


            foreach (string key in data.Keys)
            {
                if (key.Equals("LoggedIn"))
                {
                    try
                    {
                        User u = (User)Serializing.Deserialize(data["LoggedIn"]);
                        if (!UsersOnline.ContainsKey(u.GetUserId()))
                        {
                            if (User.ValidateUser(u.GetUserId(), u.GetPassword(), con))
                            {
                                /*CHANGE 1.2.10 00:14*/
                                lock (UsersOnline)
                                {
                                    UsersOnline.Add(u.GetUserId(), e.ClientSocket);
                                    string ip = e.ClientSocket.RemoteEndPoint.ToString().Split(':')[0];
                                    Utility.Log.Write("UserLog.log", u.GetUserId() +
                                        " Logged In From Ip " + ip);
                                }
                                /*CHANGE 1.2.10 00:14*/
                            }
                            else
                            {
                                server.BlockIp(e.ClientSocket);
                                return;
                            }
                        }
                        else
                        {
                            Stream tmpStream = Serializing.Serialize("Same User");
                            tmpStream = ZipNEncrypt.Zip(new string[] { key + "ERROR_SameUser" },
                                        new Stream[] { tmpStream }, pass);
                            server.Send(tmpStream, e.ClientSocket);
                            return;
                        }
                    }
                    catch (Exception) { }
                    return;
                }
                else if (key.Equals("chat"))
                {
                    string ip = e.ClientSocket.RemoteEndPoint.ToString().Split(':')[0];
                    string message = ip + " : "+ (string)Serializing.Deserialize(data[key]);
                    OnChatDataReceive(message);
                    return;
                }
                string[] str = key.Split('_');
                Stream dataStream = null;
                object obj = null;
                try
                {
                    if (str[0].StartsWith("Get"))
                    {
                        if (str[0].Equals("GetData"))
                        {
                            string query = (string)Serializing.Deserialize(data[key]);
                            obj = con.GetData(query);
                        }
                        else if (str[0].Equals("GetColumn"))
                        {
                            string query = (string)Serializing.Deserialize(data[key]);
                            string[] tmp = query.Split('%');
                            obj = con.GetColumn(tmp[0], tmp[1]);
                        }
                        else if (str[0].Equals("GetColumnDistrinctValue"))
                        {
                            string query = (string)Serializing.Deserialize(data[key]);
                            string[] tmp = query.Split('%');
                            obj = con.GetColumnDistrinctValue(tmp[0], tmp[1]);
                        }
                    }
                    else
                    {
                        lock (this)
                        {
                            if (str[0].Equals("ExecuteUpdate"))
                            {
                                if (str[1].Equals("Query"))
                                {
                                    Query query = (Query)Serializing.Deserialize(data[key]);
                                    obj = con.ExecuteUpdate(query);
                                }
                                else if (str[1].Equals("String"))
                                {
                                    string query = (string)Serializing.Deserialize(data[key]);
                                    obj = con.ExecuteUpdate(query);
                                }
                            }
                            else if (str[0].Equals("ExecuteBatchUpdate"))
                            {
                                if (str[1].Equals("Query"))
                                {
                                    Query[] query = (Query[])Serializing.Deserialize(data[key]);
                                    obj = con.ExecuteBatchUpdate(query);
                                }
                                else if (str[1].Equals("String"))
                                {
                                    string[] query = (string[])Serializing.Deserialize(data[key]);
                                    obj = con.ExecuteBatchUpdate(query);
                                }
                            }
                            else if (str[0].Equals("ExecutrInsert"))
                            {
                                Query query = (Query)Serializing.Deserialize(data[key]);
                                obj = con.ExecutrInsert(query);
                            }
                        }
                    }
                    dataStream = Serializing.Serialize(obj);
                    dataStream = ZipNEncrypt.Zip(new string[] { key },
                                new Stream[] { dataStream }, pass);
                }
                catch (Exception ex)
                {
                    dataStream = Serializing.Serialize(ex.Message);
                    dataStream = ZipNEncrypt.Zip(new string[] { key + "_ERROR" },
                                new Stream[] { dataStream }, pass);
                }
                server.Send(dataStream, e.ClientSocket);
            }
        }
        private void OnDataSendToUser(Utility.Network.ServerEventArguments e)
        {
        }
        private void OnUserDisconnect(Utility.Network.ServerEventArguments e)
        {
            //System.Windows.Forms.MessageBox.Show("Disconnected");
            string ip = e.ClientSocket.RemoteEndPoint.ToString().Split(':')[0];
            /*CHANGE 1.2.10 00:14*/
            lock (unAuthenticatedIps)
            {
                if (unAuthenticatedIps.ContainsKey(ip))
                    unAuthenticatedIps.Remove(ip);
            }
            lock (UsersOnline)
            {
                foreach (string key in UsersOnline.Keys)
                    if (UsersOnline[key].Equals(e.ClientSocket))
                    {
                        Utility.Log.Write("UserLog.log", key + " Logged Out From Ip " + ip);
                        UsersOnline.Remove(key);
                        break;
                    }
            }
            /*CHANGE 1.2.10 00:14*/
        }
        private void OnUserBlocked(Utility.Network.ServerEventArguments e)
        {
            string ip = e.ClientSocket.RemoteEndPoint.ToString().Split(':')[0];
            Utility.Log.Write("UserLog.log", "Blocked For Illegal Access From Ip " + ip);
        }
        public void Send(Stream dataStream)
        {
            foreach (string key in UsersOnline.Keys)
            {
                try
                {
                    server.Send(dataStream, UsersOnline[key]);
                }
                catch (Exception) { }
            }
        }
        public void Send(Stream dataStream, Socket client)
        {
            try
            {
                server.Send(dataStream, client);
            }
            catch (Exception) { }
        }
        /*changed*/
        public bool AddUser(string userId, Socket socket)
        {
            if (UsersOnline.ContainsKey(userId)) return false;
            UsersOnline.Add(userId, null);
            return true;
        }
        public void RemoveUser(string userId)
        {
            if (!UsersOnline.ContainsKey(userId) || UsersOnline[userId] != null) return;
            UsersOnline.Remove(userId);
        }
    }
}

Теперь я не уверен, что правильно использую блокировку. Пожалуйста, дайте мне несколько советов. Спасибо.

Answer 1

Полагаю, вы читаете намного больше, чем пишете? Если это так, ReaderWriterLockSlim может быть более подходящим для уменьшения блокировки (возьмите чтение, когда вы просто хотите проверить ключ, и напишите, чтобы манипулировать данными).

Я имею в виду, что сначала вы можете дважды проверить блокировку с чтением, а затем, если она не удастся, взять блокировку записи, снова проверить и добавить, если необходимо.

Также - lock(this) обычно осуждается; предпочтительнее иметь отдельный объект блокировки.

Обратите внимание, что для вступления в силу все доступ должен учитывать блокировку; Есть места, где UsersOnline заблокирован, и некоторые места, где он доступен без блокировки, например; эти вторые случаи могут взорваться в липком беспорядке.

Например:

if (!UsersOnline.ContainsKey(u.GetUserId()))
{
    if (User.ValidateUser(u.GetUserId(), u.GetPassword(), con))
    {
        /*CHANGE 1.2.10 00:14*/
        lock (UsersOnline)
        {
            UsersOnline.Add(u.GetUserId(), e.ClientSocket);

В приведенном выше примере, если возможно, что два потока смотрят на UsersOnline, то вы уже потерпели неудачу, пытаясь ContainsKey без блокировки. Если другой поток изменяет состояние при этом .... boom .

Answer 2

Прежде всего, ваш код вообще не является потокобезопасным. В вашем коде вы блокируете только операции изменения (Удалить, Добавить), но также вы должны заблокировать весь доступ к общим полям. На самом деле этот код не является потокобезопасным вообще. Я думаю, что в этом случае ReaderWriterLockSlim - будет лучшим выбором.

Во-вторых. замок (это) - очень идея кровати. Вы должны использовать специальные объекты для этого.

Наконец, я думаю, что ваш код грязный и сложный для понимания. Может быть, ваш класс решит много разных задач. Возможно, вам следует извлечь некоторую логику в отдельные классы (например, создать защищенные словари как отдельные классы) или что-то еще.

Пример использования ReaderWriterLockSlim :

someSharedResource;
someSharedResourceRWLock = new ReaderWriterLockSlim();

Код чтения:

try
{
   someSharedResourceRWLock.EnterReadLock();
   //access to someSharedResource for reading
}
finally
{
   someSharedResourceRWLock.ExitReadLock();
}

Некоторый код написания:

try
{
   someSharedResourceRWLock.EnterWriteLock();
   //access to someSharedResource for modifications
}
finally
{
   someSharedResourceRWLock.ExitWriteLock();
}

Answer 3

Вы используете его правильно некоторое времени. unAuthenticatedIps защищен правильно, а UsersOnline нет. Давайте рассмотрим два параллельных потока, проходящих через ваш код:

                                             Thread A           Thread B
                                             --------           --------
if (!UsersOnline.ContainsKey(u.GetUserId())  Statement's true   Statement's true
{
    lock (UsersOnline)                       Get lock           Block
    {
        UsersOnline.Add                      UsersOnline.Add
    }                                        Release Lock
}                                                               Get lock
                                                                UsersOnline.Add
                                                                Release lock   

Обратите внимание, что оба потока A и B изменяют словарь UsersOnline. Эта структура будет правильно защищать этот объект:

if (!UsersOnline.ContainsKey(u.GetUserId()))
{
    if (User.ValidateUser(u.GetUserId(), u.GetPassword(), con))
    {
        lock (UsersOnline)
        {
            // Note the additional check in case another thread
            // added this already
            if (!UsersOnline.ContainsKey(u.GetUserId())
            {
                UsersOnline.Add(u.GetUserId(), e.ClientSocket);
                // ...
            }
        }
    }
    else
    {
        server.BlockIp(e.ClientSocket);
        return;
    }
}

Что касается последней блокировки (lock (this)), я пока не понимаю, зачем вам это нужно. str и obj являются локальными переменными, поэтому вам не нужно беспокоиться об их изменении отдельными потоками. И, как уже говорилось, блокировка this не рекомендуется.

Answer 4

Советы, данные до сих пор, были великолепны, но у меня есть предложение по дизайну, которое вы можете рассмотреть. Заменить это:

private Dictionary<string, Socket> UsersOnline; 

с настраиваемым словарем безопасного потока

private ThreadSafeDictionary<string, Socket> UsersOnline

Если это подходит для ваших нужд, это был бы хороший способ отделить бизнес-логику от логики потоков.