Implementing a thread safe log class with simple functionality I have finally assembled an attempt to create a thread safe logging class and also ensured by file flags that file should not be accessed by different processes at same time. This classed will be inside DLL. Below is the implementation,

   static class GMLogger
    {

       private static List&lt;string&gt; _buffer = new List&lt;string&gt;();
       private static int _maxBufferSize = 25;  // number of messages  permitted in the buffer at a time
       public static string _logDir = "C:\\Logs";
       private static readonly object _syncObject = new object();

       // Log message
        public static  void Log(string logMessage)
        {
            try
            {
                lock (_syncObject)
                {
                    _buffer.Add(logMessage);

                    Save(_buffer);
                }
            }
            catch (Exception ex)
            {
                throw;
            }
        }

       // Save buffer if needed
        private static void Save(List&lt;string&gt; buffer)
        {
            if (buffer.Count &gt; _maxBufferSize)
            {
                // Write to file
                if (!_logDir.EndsWith("\\")) _logDir += "\\"; 
                DirectoryInfo di = Directory.CreateDirectory(_logDir);
                var todaysLogFilePath = Path.Combine(_logDir,
                                                       ("Log" + DateTime.Now.ToString("yyyy-MMMM-dd") + ".txt"));
                using (FileStream aFile = new FileStream(todaysLogFilePath, FileMode.Append, FileAccess.Write, FileShare.None))
                using (StreamWriter sw = new StreamWriter(aFile))
                {
                    for (int i = 0; i &lt; buffer.Count(); i++)
                    {
                        sw.WriteLine(buffer[i]);
                    }
                }

                // Clear buffer
                buffer.Clear();
            }



    }
}


Please note I am a beginner first of all, and if this class achieves the simple functionality of writing simple lines to file I am OK with it.

Of course you can comment also about other things which are in the context of CodeReview site. But please not too advanced feedback since as I said I am beginner.

Second, here are some specific questions I have about it:


Have I used the lock correctly?
This class maybe used by many threads, will I have performance problems because of thread waiting for each other? NOTE: I log only ERRORS so even if many threads are using it, not many errors should be occurring right? Also note how because of performance I have a cache for log messages.
Finally, only problem I see if different programs use this class to write to same file, how can I distinguish inside logs which program or process made an entry? How is this solved typically? (I assume one process can write to this file if another finished writing, isn't it?)


PS. This is all the functionality I want, I don't plan to extend this class. So please no recommendations about 3rd party libs. If my class achieves above points well, I am ok with it. Or if you see some problem with it, please tell. 

And in general I am interested if its safe to use my class because some people always say to not create your own logging class. Is it ok if I use this class? I see people creating log classes such as here http://www.codeproject.com/Tips/585796/Simple-Log, so what is problem if I created one as above?
One of the things you can do to get some performance back (albeit not a lot, but still more than none) is rewrite Save to have one responsibility.

As it stands now, Save has several responsibilities. It's responsible for:


Checking if the buffer is of sufficient length to be saved.
Validating that _logDir ends with a \.
Creating the _logDir directory.
Coming up with the filename for the log file.
Saving the buffer to disk.
Clearing the buffer after saving.


The Save method should only be responsible for saving (and maybe clearing) the buffer. If you move the if check outside to the Log method, you remove a (minor) bit of overhead.

A better style would be to create a method SaveIfFull which does the if-check, then calls Save which actually saves the buffer.

You have a:

public static string _logDir = "C:\\Logs";


Which is generally frowned upon. The best-practice for public members is PascalCase with no leading characters. So, instead, it should be:

public static string LogDir = "C:\\Logs";


Never omit braces from if statements, and never one-line them. Adding braces won't prevent bugs, but it will help prevent bugs.

if (!_logDir.EndsWith("\\")) _logDir += "\\";


Should be rewritten as:

if (!_logDir.EndsWith("\\"))
{
    _logDir += "\\";
}


Likewise, you should make _logDir (which we to LogDir) a property, with a private backing field.

private static string _logDir = "C:\\Logs\\";
public static string LogDir
{
    get
    { 
        return _logDir;
    }
    set
    { 
        if (string.IsNullOrWhiteSpace(value))
        { 
            throw new ArgumentException("The specified value must not be null, whitespace, or an empty string.", "value");
        }

        _logDir = value;

        if (!_logDir.EndsWith("\\"))
        {
            _logDir += "\\";
        }
    }
}


You can also add other validation logic in there as you need, but this way it helps separate responsibilities further.

To get rid of some of the other additional responsibilities Save has, we can consider a method GetLogFileName(DateTime loggedAt) which would return the filename, then Save would call string todayFileName = GetLogFileName(DateTime.Now) or string todayFileName = GetLogFileName(DateTime.UtcNow) as appropriate.



If you have C#6.0 a few of these tasks become easier/simpler:


Path.Combine(_logDir, ("Log" + DateTime.Now.ToString("yyyy-MMMM-dd") + ".txt")); simplifies to: Path.Combine(_logDir, $"Log{DateTime.Now.ToString("yyyy-MMMM-dd")}.txt");
You can replace "value" at the end of ArgumentException with nameof(value). (Which allows you to refactor that message cleanly.)




As far as your primary concerns:


Yes, the idea of the lock is to mark a shared resource as "in-use" so that threads know to wait on each other before they can proceed. You could, however, replace it with a bool instead of an object for smaller overhead.
You could have performance implications by the lock, but that's the downside of multithreading: somewhere you will have a bottleneck from context-switching, et al.
That is beyond the scope of Code Review.




Another concern: if there is a substantial delay between messages, it may occur that the log is never written to before the application is closed (or messages are left in the buffer), this can be alleviated by scheduling a Timer to save the log at periodic intervals as well, so that if there is a substantial delay the log is saved anyway.