c# - ここでロックを適切に使用していますか？より少ないコードをラップする必要がありますか?

Question

複数のスレッドからアクセスされるこのコードを継承しました。2 つのロックを紹介しましたが、他に知っておくべきことがあるでしょうか。マルチスレッド アプリケーションで作業した経験はほとんどありません。

namespace Helpers.Security
{
    public static class Encryption
    {
        #region "Random Numbers"
        static readonly int[] _randData = {
            //A gigantic list of numbers...
        };
        #endregion

        private static int _randIdx = 0;

        private static readonly object _encryptLock = new object();
        private static readonly object _decryptLock = new object();

        //HG 2009-JUN-11 - Added Reverse Methods from PF's Merge updates in [CDataServerBootStrapper]
        public static string EncryptStringReverse(string c_string)
        {
            return Encrypt(ReverseString(c_string));
        }
        public static string DecryptStringReverse(string c_string)
        {
            return Decrypt(ReverseString(c_string));
        }
        private static string ReverseString(string inputString)
        {
            string result = string.Empty;
            for (int pos = inputString.Length - 1; pos >= 0; pos--)
                result += inputString[pos];

            return result;
        }

        public static string Encrypt(string c_string)
        {
            if (c_string == null || c_string.Equals(string.Empty)) return string.Empty;
            int[] sasc = new int[224];
            char[] chash = new char[224];
            bool isExisting = false;
            string encstr = "";
            int sl = c_string.Length;

            lock (_encryptLock)
            {
                _randIdx = 0;

                for (int v = 0; v < 223; v++)
                {
                    sasc[v] = '\0';
                }
                for (int cl = 0; cl < sl; cl++)
                {
                    for (int a = 0; a < 223; a++)
                    {
                        int rnum = _randData[_randIdx++];
                        for (int y = 0; y < 223; y++)
                        {
                            if (sasc[y] == rnum)
                            {
                                isExisting = true;
                            }
                        }
                        if (isExisting == false)
                        {
                            sasc[a] = rnum;
                            chash[a] = (char) rnum;
                        }
                        else
                            a--;
                        isExisting = false;
                    }
                    chash[223] = '\0';
                    string strhash = new string(chash);
                    for (int v = 0; v < 223; v++)
                    {
                        sasc[v] = '\0';
                    }
                    encstr = encstr + strhash[c_string[cl] - 30];
                }
            }

            // Convert the wide-character string to multibyte string
            string sWholeHex = "";
            foreach (char c in encstr)
            {
                byte val = (byte) c;

                sWholeHex += val.ToString("X2");
            }

            return (sWholeHex.Trim().Replace("\0", ""));
        }

        public static string Decrypt(string c_string)
        {
            if (c_string == null || c_string.Equals(string.Empty)) return string.Empty;

            string szTemp = c_string;
            int nCtr = 0;
            byte[] byToDecrypt = new byte[1024];
            char[] chash = new char[223];
            char[] cencstr = new char[5000];
            int[] sasc = new int[223];
            bool isExisting = false;

            lock (_decryptLock)
            {
                for (int b = 0; b < 1024; b++)
                    byToDecrypt[b] = 0;

                int r;
                string sToDecrypt = string.Empty;
                for (r = 0; r < szTemp.Length - 1; r += 2)
                {
                    byte b2 = 0;
                    char c = szTemp[r];
                    if (c >= '0' && c <= '9')
                        b2 += (byte) (c - '0');
                    else if (c >= 'A' && c <= 'Z')
                        b2 += (byte) (c - 'A' + 10);

                    b2 *= 16;
                    c = szTemp[r + 1];
                    if (c >= '0' && c <= '9')
                        b2 += (byte) (c - '0');
                    else if (c >= 'A' && c <= 'Z')
                        b2 += (byte) (c - 'A' + 10);

                    byToDecrypt[nCtr++] = b2;
                    sToDecrypt += (char) b2;
                }

                _randIdx = 0;

                int sl = sToDecrypt.Length;

                for (int v = 0; v < 223; v++)
                {
                    sasc[v] = '\0';
                }
                int cl;
                for (cl = 0; cl < sl; cl++)
                {
                    for (int a = 0; a < 223; a++)
                    {
                        int rnum = _randData[_randIdx++];
                        for (int y = 0; y < 223; y++)
                        {
                            if (sasc[y] == rnum)
                            {
                                isExisting = true;
                            }
                        }
                        if (isExisting == false)
                        {
                            sasc[a] = rnum;
                            chash[a] = (char) rnum;
                        }
                        else
                        {
                            a--;
                        }
                        isExisting = false;
                    }
                    string strhash = new string(chash);
                    int v;
                    for (v = 0; v < 223; v++)
                    {
                        if (sToDecrypt[cl] == strhash[v])
                            cencstr[cl] = (char) ((byte) (v + 30));
                    }

                    for (v = 0; v < 223; v++)
                    {
                        sasc[v] = 0;
                    }
                }
                cencstr[cl] = '\0';
            }

            string encstr = new string(cencstr);

            return (encstr.Trim().Replace("\0", ""));
        }
    }
}

私の唯一の考えは、ここで多くの作業が行われているため、すべての作業をロックでラップして、ロックの問題を他の場所にプッシュしないようにする必要があるということでした。そうは言っても、グローバルにアクセスできる変数は _randIdx と _randData の 2 つだけです。それが何をロックするかを決定する要因になるかどうかはわかりません。

Answer 1

That depends on what kind of thread safety you need.

_randIdx can be both set and got by two different threads because your encrypt and decrypt method use different objects to lock around. You're only performing reads on the _randData array in each lock, so that's a non issue.

It seems like it would be trivially easy to make this non lock required by simply using a temporary index local to just that one method. Whether that is valid or not is up to you.

Answer 2

You will need to use the same lock to protect the _randIdx global variable in both the encryption and the decryption. _randData is not modified, so no need to protect it specifically. As all methods of the class are static, you don't have any other members to protect from simulateous access/modification

Answer 3

Attilaは順調ですが、これらのアイデアを説明するために、さらにいくつかの単語を使用します。

2つのロックターゲット（_encryptLockと_decryptLock）があります。これらのロックは、異なるスレッドで同時に入力できます。これは、暗号化メソッドが_randidxを0に設定できることを意味しますが、復号化メソッドはそれを使用していました（悪い）。 保護されたインスタンスのセットごとに1つのロックターゲットのみを使用します。

_randDataを変更していないため、実際には_randDataを保護するためのロックは必要ありません。複数のスレッドが同じ配列から安全に読み取ることができます。_randDataのインスタンスも値型であるため、保護する必要はありません。配列にアクセスする人は誰でもコピーを取得し、元のインスタンスを変更しません。

_randidxを2つのメソッドスコープ変数に変更した場合、_randidxを保護するためのロックは必要ありません。1つのスレッドにのみ属するものは安全です。

ロックで保護する必要のあるインスタンスがないため、ロックがない可能性があります。

Answer 4

ブロック内で実行される「作業」の量を最小限に抑える必要がありlockます。ロック内の作業を別のオブジェクトにリファクタリングすることをお勧めします（経由[Array.Copy(...)][1]）_randData。

実際に変更 _randDataしていますか？そうでなければ、私はロックする理由がわかりません。また、2つの異なるロックを使用しています。1つは暗号化用、もう1つは復号化用です。_randData2つのスレッドからアクセスできるようにすることは意図的ですか？2つのロックオブジェクトがあると、これが発生します。