Am I using the lock correctly? Should I try to wrap less code with it?

I just inherited this code accessed by multiple threads. I just introduced two castles, but I wonder if there is anything else I need to know. I do not have much experience in multithreaded applications.

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 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", ""));
        }
    }
}

My only thoughts were that there is a lot of work here, so I have to wrap all the work in a lock to make sure that I'm not just pushing the lock problem elsewhere. In this case, only two variables are globally available ... _randIdx and _randData. I'm not sure if this plays an important role in deciding whether to block.

+3
source share
4 answers

, .

_randIdx , , . _randData , .

, lock , , . , .

+2

_randIdx , . _randData , . static, /

+1

, , :

(_encryptLock _decryptLock). . , Encrypt _randidx 0, Decrypt (). .

_randData, . . _randData , - , , .

_randidx, . , , .

, , .

+1

You should minimize the amount of "work" performed in the block lock. I would recommend reorganizing the work inside the castle to another object, possibly passing (through [Array.Copy(...)][1]) _randData.

Are you really cheating _randData ? If not, I see no reason to block. You also use 2 different locks - one for encryption and one for decryption. Do you intentionally have access to _randDatafrom 2 threads? Having 2 lock objects will allow this to happen.

0
source

All Articles