Static thread safe configuration class

Question

I wrote this class. I would be very interested in your feedback how much thread safe this class is. I know users of this class must still use some kind of synchronization when using this class, but I am ok with it, they will do such synchronization.

To save your time and energy, I am more interested in thread safety related feedback of this code, but you can also comment on other aspects of code too, if you wish.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace dppClientModuleNET
{
 public enum ApplicationMode
 {
 SOFTWARE_HSM = 0,
 POSTER_VER = 1
 }
 /// <summary>
 /// This static class is used to manage parameters of this DLL.
 /// You usually initialize it only once using the Init method,
 /// and then mainly query the class for different parameter
 /// values. Properties are mainly only readable.
 /// You can also deinitialize this class.
 /// This class has been written with thread safety in mind-
 /// use with care.
 /// </summary>
 static class DppModuleParameters
 {
 private static bool m_isInited = false; // Is initialized or not?
 static readonly object m_locker = new object(); // Locker
 private static ushort m_softwareVersion = 0x0250; // Software version
 private static ApplicationMode m_applicationMode = ApplicationMode.SOFTWARE_HSM; // Build type
 private static string m_logDirPath = ""; // Log directory
 private static uint m_connectTimeoutMS = 0; // Connect timeout 
 private static uint m_responseTimeoutMS = 0; // Response timeout
 private static uint m_indexHost = 0; // Host index
 private static int m_gComPortNumber = 0; // Com port number - this was used as global variable in C++
 private static List<SocketStructure> m_HostAddresses = new List<SocketStructure>(); // List of host addresses
 private static string m_KeysFileName = ""; // Path to the keys file
 private static List<Key_t> m_DecryptedKeys = new List<Key_t>(); // Array of decrypted keys 
 // Getter: Is module initialized?
 public static bool isInited()
 {
 lock (m_locker)
 {
 return m_isInited;
 }
 }
 // Get software version
 public static int GetSoftwareVersion()
 {
 lock (m_locker)
 {
 if (m_isInited == false)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
 }
 return m_softwareVersion;
 }
 }
 // Get log path
 public static string GetLogPath()
 {
 lock (m_locker)
 {
 if (m_isInited == false)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
 }
 return m_logDirPath;
 }
 }
 // Get connect timeout
 public static uint GetConnectTimeout()
 {
 lock (m_locker)
 {
 if (m_isInited == false)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
 }
 return m_connectTimeoutMS;
 }
 }
 // Get build type 
 public static ApplicationMode GetBuildMode()
 {
 lock (m_locker)
 {
 if (m_isInited == false)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
 }
 return m_applicationMode;
 }
 }
 // Get response timeout
 public static uint GetResponseTimeout()
 {
 lock (m_locker)
 {
 if (m_isInited == false)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
 }
 return m_responseTimeoutMS;
 }
 }
 // Get index host
 public static uint GetIndexHost()
 {
 lock (m_locker)
 {
 if (m_isInited == false)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
 }
 return m_indexHost;
 }
 }
 // Set index host
 public static void SetIndexHost(uint host)
 {
 lock (m_locker)
 {
 if (m_isInited == false)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
 }
 m_indexHost = host;
 }
 }
 // Get COM port number
 public static int GetComPortNumber()
 {
 lock (m_locker)
 {
 if (m_isInited == false)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
 }
 return m_gComPortNumber;
 }
 }
 // Get list of host addresses
 // NOTE: Makes a deep copy of the host address array and returns that
 public static List<SocketStructure> GetHostAddressesArray()
 {
 lock (m_locker)
 {
 // Make a deep copy of the list of the host addresses
 List<SocketStructure> tmp = new List<SocketStructure>();
 for (int i = 0; i < m_HostAddresses.Count(); i++)
 {
 SocketStructure s = new SocketStructure();
 s.IP = m_HostAddresses[i].IP;
 s.port = m_HostAddresses[i].port;
 tmp.Add(s);
 }
 return tmp;
 }
 }
 // Getter for keys file name
 public static string GetKeysFileName()
 {
 lock (m_locker)
 {
 if (m_isInited == false)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_NOT_INITIALIZED), "Please initialize module parameters class first");
 }
 return m_KeysFileName;
 }
 }
 // GetKeys
 // NOTE: Makes a deep copy of the keys array and returns that
 public static List<Key_t> GetKeysArray()
 {
 lock (m_locker)
 {
 // Make a copy of the list of the keys
 List<Key_t> tmp = new List<Key_t>();
 for (int i = 0; i < m_DecryptedKeys.Count(); i++)
 {
 Key_t s = new Key_t();
 s.KeyName = m_DecryptedKeys[i].KeyName;
 for (int j = 0; j < 8; j++)
 {
 // Copy each key separately
 s.MasterKey[j] = m_DecryptedKeys[i].MasterKey[j];
 s.SessionKey[j] = m_DecryptedKeys[i].SessionKey[j];
 }
 tmp.Add(s);
 }
 return tmp;
 }
 }
 /// <summary>
 /// Initialize fields of the DppModuleParameters class. Initialization should be done once.
 /// Otherwise you will get exception.
 /// </summary>
 /// <param name="errorInfo">[OUT] Error info structure</param>
 /// <param name="logDirPath">log path</param>
 /// <param name="hsm">Hardware security module parameter </param>
 /// <param name="hostAddresses">Prepaid Server addresses (";"-separated example: "x.x.x.x:yyyy;second.server.name:yyyy")</param>
 /// <param name="connectTimeoutMS"> Connection timeout in ms (0-default value: 15000ms) </param>
 /// <param name="responseTimeoutMS"> Server response timeout in ms (0-default value: 45000ms) </param>
 /// <param name="softwareVersion"> [OUT] Module version </param>
 /// <param name="indexTCIP">Index to which TCP host to connect; default value is 0</param>
 /// <returns>status</returns>
 public static int Initialize(ref DppErrorInfo_t errorInfo,
 string logDirPath,
 string hsm,
 string hostAddresses,
 uint connectTimeoutMS,
 uint responseTimeoutMS,
 ref ushort? softwareVersion,
 uint indexTCIP,
 ApplicationMode buildmode)
 {
 // Lock
 lock (m_locker)
 {
 try
 {
 try
 {
 // We don't allow this structure to be null
 if (errorInfo == null)
 return DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG);
 // Just clean the error structure
 errorInfo.Code = 0;
 errorInfo.ActionCode = 0;
 errorInfo.SysCode = 0;
 errorInfo.Description = "";
 errorInfo.DescriptionFromServer = "";
 errorInfo.Code = DppGlobals.dppERR_SUCCESS;
 // Store build mode
 m_applicationMode = buildmode;
 // .......................
 // Module parameter object already initialized?
 if (m_isInited)
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_ALREADY_INITIALIZED), "Parameters module already initialized. Deinitialize first please.");
 // Pass out software version if out param is not null
 if (softwareVersion != null)
 softwareVersion = m_softwareVersion;
 // Is log directory empty? throw an exception
 if (String.IsNullOrEmpty(logDirPath))
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "Log path not specified");
 // List of host addresses string is null or empty?
 if (String.IsNullOrEmpty(hostAddresses))
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "Host list not specified");
 // If HSM is NULL throw a module error exception
 // if it is empty string we are Okay
 if (hsm == null)
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "HSM not given");
 // Extract HSM string and store COM port number in instance variable
 if (TranslateHSM(hsm) < 0)
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "Wrong HSM specified");
 // ....................... 
 // Parse host addresses and store them
 string[] firstSplit = hostAddresses.Split(';');
 for (int i = 0; i < firstSplit.Length; i++)
 {
 string[] secondSplit = firstSplit[i].Split(':');
 if (secondSplit.Length != 2)
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "ParseHostAddresses: List of socket addresses is in not correct format");
 SocketStructure sockstruct = new SocketStructure();
 sockstruct.IP = secondSplit[0].Trim();
 sockstruct.port = Int32.Parse(secondSplit[1]);
 m_HostAddresses.Add(sockstruct);
 }
 // List of host addresses empty?
 if (m_HostAddresses.Count() == 0)
 {
 throw new DppModuleException(DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_INVALID_ARG), "Host address list not specified");
 }
 // Set time out parameters
 m_connectTimeoutMS = (connectTimeoutMS != 0 ? connectTimeoutMS : DppGlobals.ConnectTimeOutDefault);
 m_responseTimeoutMS = (responseTimeoutMS != 0 ? responseTimeoutMS : DppGlobals.ResponseTimeOutDefault);
 // Set log dir path of the logger, also store the path 
 m_logDirPath = logDirPath;
 DppLogger.LogDirectory = logDirPath;
 // Software HSM?
 if (m_applicationMode != ApplicationMode.POSTER_VER)
 {
 // Get name of the key file
 // Note: Since module is not initialized yet, we need to pass along some parameters
 // otherwise other classes can't use getters to access them
 DppModuleParameters.GetKeyFileName(buildmode, m_gComPortNumber);
 // Read key file
 DppModuleParameters.ReadKeyFile();
 }
 m_indexHost = indexTCIP;
 // Mark as initialized - this is final step
 m_isInited = true;
 }
 // Catch module error
 catch (DppModuleException ex)
 {
 ex.FillErrorStruct(ref errorInfo);
 }
 // Catch OS error
 catch (DppOSException ex)
 {
 ex.FillErrorStruct(ref errorInfo);
 }
 // Server error
 catch (DppServerException ex)
 {
 ex.FillErrorStruct(ref errorInfo);
 }
 // Catch general exception
 catch (Exception ex)
 {
 DppUtilities.FillErrorStructWithGeneralException(ref ex, ref errorInfo);
 }
 }
 catch (Exception ex)
 {
 // Some unexpected exception occured probably in the catch clauses, return error code
 return DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_UNKNOWN);
 }
 // Return the module code from the data structure.
 return errorInfo.Code;
 }
 }
 /// <summary>
 /// Deinitialize function
 /// </summary>
 /// <param name="errorInfo">[OUT] error structure</param>
 /// <param name="pIndexTCIP">[IN/OUT] index of host</param>
 /// <returns>status</returns>
 public static int DeInit(ref DppErrorInfo_t errorInfo, ref uint? pIndexTCIP)
 {
 // Lock
 lock (m_locker)
 {
 try
 {
 try
 {
 // Just clean the error structure
 errorInfo.Code = 0;
 errorInfo.ActionCode = 0;
 errorInfo.SysCode = 0;
 errorInfo.Description = "";
 errorInfo.DescriptionFromServer = "";
 // Pass out index
 if (pIndexTCIP != null)
 pIndexTCIP = m_indexHost;
 m_indexHost = 0;
 // Clear out log directory
 m_logDirPath = "";
 DppLogger.LogDirectory = "";
 // Clear out other parameters
 m_HostAddresses.Clear();
 m_connectTimeoutMS = 0;
 m_responseTimeoutMS = 0;
 // Software HSM?
 if (m_applicationMode != ApplicationMode.POSTER_VER)
 {
 // Yes, clear decrypted keys
 m_DecryptedKeys.Clear();
 }
 m_isInited = false;
 }
 catch (DppModuleException ex)
 {
 ex.FillErrorStruct(ref errorInfo);
 }
 catch (DppOSException ex)
 {
 ex.FillErrorStruct(ref errorInfo);
 }
 catch (DppServerException ex)
 {
 ex.FillErrorStruct(ref errorInfo);
 }
 catch (Exception ex)
 {
 DppUtilities.FillErrorStructWithGeneralException(ref ex, ref errorInfo);
 }
 }
 catch (Exception ex)
 {
 // Some unexpected exception occured probably in the catch clauses, return error code
 return DppUtilities.MAKE_HRESULT(DppGlobals.dppERR_UNKNOWN);
 }
 return errorInfo.Code;
 }
 }
 // Extract COM port number from supplied string. 
 // Supplied string should be in form "TYPE=COM;NUMBER=3"
 private static int TranslateHSM(string hsm)
 {
 if (m_applicationMode == ApplicationMode.SOFTWARE_HSM)
 {
 // Exit if this is software HASP build
 return 0;
 }
 // Perform splitting and extraction
 string[] split1 = hsm.Split(';');
 if (split1.Length == 2)
 {
 string[] splitTmp1 = split1[0].Split('=');
 if (splitTmp1[1] != "COM")
 return -1;
 string[] splitTmp2 = split1[1].Split('=');
 if (splitTmp2[0] != "NUMBER")
 return -1;
 // Extract the port number
 m_gComPortNumber = int.Parse(splitTmp2[1]);
 }
 else
 {
 return -1;
 }
 return 0;
 }
 /// <summary>
 /// Parse keys from the key file
 /// </summary>
 /// <param name="inBuffer">byte array representation of a text file which contains keys stored as hex string on separate lines</param>
 /// <param name="bufferSize">size of the byte array</param>
 private static void ParseTextFile(byte[] inBuffer, uint bufferSize)
 {
 string line = "";
 using (Stream stream = new MemoryStream(inBuffer))
 {
 using (StreamReader reader = new StreamReader(stream))
 {
 while (true)
 {
 // Read text file line by line
 line = reader.ReadLine();
 if (line == null)
 {
 break;
 }
 string[] parameters = line.Split(';');
 if (parameters.Length == 3)
 {
 Key_t k = new Key_t();
 // Copy key name
 k.KeyName = parameters[0];
 // Copy master key
 byte[] mk = DppUtilities.HexStringToByteArray(parameters[1]);
 Array.Copy(mk, k.MasterKey, 8);
 // Copy session key
 byte[] sk = DppUtilities.HexStringToByteArray(parameters[2]);
 Array.Copy(sk, k.SessionKey, 8);
 // Add to the global array of keys
 m_DecryptedKeys.Add(k);
 }
 }
 }
 }
 }
 /// <summary>
 /// Retrieve path of the file where keys are stored
 /// </summary>
 private static void GetKeyFileName(ApplicationMode b, int compport)
 {
 // Get folder where DLL resides and make sure path is terminated
 string dllFolder = System.IO.Path.GetDirectoryName(new System.Uri(System.Reflection.Assembly.GetExecutingAssembly().CodeBase).LocalPath);
 if (!dllFolder.EndsWith("\\")) dllFolder += "\\";
 // Call to get serial number function
 DppPosterApi.GetSerialNumber(ref dllFolder, 0 /* should be size of string but not needed in C#*/, 0, DppGlobals.HS_READ_TIMEOUT, 0, b, compport);
 // Store the result in a global variable
 m_KeysFileName = dllFolder + ".enc";
 }
 /// <summary>
 /// Read the key file and get keys out of it.
 /// </summary>
 /// <returns></returns>
 private static int ReadKeyFile()
 {
 // Clear the global keys array 
 m_DecryptedKeys.Clear();
 // Software HASP?
 if (m_applicationMode != ApplicationMode.SOFTWARE_HSM)
 {
 if (m_gComPortNumber <= 0) throw new Exception("Wrong port number");
 }
 // Open file for reading data
 using (FileStream stream = File.Open(m_KeysFileName, FileMode.Open, FileAccess.Read, FileShare.Read))
 {
 // Get file length
 long length = new System.IO.FileInfo(m_KeysFileName).Length;
 byte[] buffer = new byte[length + 1];
 // Read the file contents
 DppUtilities.ReadFileFully(stream, buffer, (int)length);
 if (m_applicationMode == ApplicationMode.SOFTWARE_HSM)
 {
 // Decrypt file contents
 DppCryptography.DecryptKeyFile(buffer, (uint)length, buffer);
 }
 // Parse keys from the text file
 ParseTextFile(buffer, (uint)length);
 }
 return 0;
 }
 }
}

Answer 1

Locking

As I read the code I can see that any public method has big lock (m_locker) which should be fine for thread safety. Any transactional usage would need another locking, but that is not problem of your static class - well, you could somehow expose the locking (e.g. creating IDisposable helper for code like using(DppModuleParameters.Locker()) transaction()) but it looks fine to me as it is.

Private members do not use locking. That is fine too.

Style

Well, looks like you are half the way between C++ and C#. Namespace starting with lowerCase, UPPER_CASE_WITH_UNDERSCORE constants (as enum in C# is more like enum class in C++, not good old enum). DppGlobals.dppERR_INVALID_ARG and DppUtilities.MAKE_HRESULT ...hmm, nothing to add - that's C++ not C#.

Second Look (added)

Well, it is thread-safe as it can be, if that is your only concern, but the whole class is... ehm, ugly, sorry. Why DeInit? Why Get/Set methods instead of properties? Why locking at all? I would rather think about singleton pattern (init once, live forever), remove locks inside simple getters (but leave the simple check to throw if you forget to call Init). Or it can be full class - just create it with proper args and get the info parsed untill you free the class.

I can see only one setter - SetIndexHost, but the m_indexHost is only used in getter and DeInit, but I guess you can use it to index some of the lists... so, why not returning readonly collections? Maybe I am missing something here (the expected usage), but I would really design it in completely different way:

1. Normal Class (not static)
2. Read-only (getters and reaonly collections)
3. Tiny view objects if you really need to change the index - create helper class that can access the collections and return the indexed value.

Something more.... immutable

Static reference to normal class

What I had in mind was static reference to the class, where static Lib TheLib = new Lib() is like calling Init and theLib = null is like DeInit and you can even have public Lib Default { get; set; } in it. But if you cannot redesign, you cannot. You got my thoughts, now it is up to you.

• Konrad Morawski: Plus the infamous m_ prefix and super-terse names (tmp, s, sjk, k, sockstruct, hsm...). I'm glad this style doesn't belong to C# "native" coding convention...
• user88911: is that what you say sufficient? lock in public methods? private methods don't use locking? I know for different kind of thread safety users of this class must also ensure synchronization
• user52292: Locking in public methods ensure, that they are properly synchronized for any single usage. Private methods are called from public methods holding the lock. Other lock would be needed for transaction not already provided by your class.
• user88911: @firda: Thanks for the feedback! I am a little but beginner with .NET - so if you don't see any functional defects, then I would leave it as it is. I can't invest now to redesign it. What do you think? Thanks again
• user88911: I rewrote this class from C++ and the C++ version had init/deinit - and it was used like that, so decided to leave it in