Is this a safe and *relatively okay* way to log certain events asynchronously?

Question

Working with:

• .NET 4.5.1
• Web Forms
• Entity Framework 6 (Context Per Request)
• IIS 8, Windows 2012 Datacenter

Main concern: Thread safety and reliability.

The project consist of a closed system that will be used by numerous types of users which will execute various actions. In our current project I've decided to implement something that probably most developers find absolutely necessary.

There were serious problems in the past deriving from the lack of even the simplest logging system that would allow us to track some common user actions, especially manipulating data.

I know there are popular logging frameworks, but I want to achieve something relatively simple that will not block the main thread.

The idea was to pull all data I need while on the main thread, because it turned out some of the data was harder to access from a separate thread, and then create a task that will take care of the database insert. My knowledge of multi-threading is limited, and this is what I came-up with.

public static class EventLogger
    {
        public static void LogEvent(int eventType, string descr, Exception ex = null)
        {
            //Get the page that is currently being executed;
            var executingPage = HttpContext.Current.CurrentHandler as Page;
            string sourcePage = executingPage != null ? executingPage.AppRelativeVirtualPath : string.Empty;

            var eventToAdd = new Event()
            {
                Date = DateTime.Now,
                EventTypeId = eventType,
                isException = ex != null ? true : false,
                ExceptionDetails = ex != null ? ex.Message : string.Empty,
                Source = sourcePage,
                UserId = UserHelper.GetUserGuid(),
                UserIP = UserHelper.GetUserIP(),
                UserName = UserHelper.GetUserName(),
                Description = descr
            };

            Task.Factory.StartNew(() => LogEventAsync(eventToAdd));
        }

        private static void LogEventAsync(Event eventToAdd)
        {
            using (var context = new PlaceholderEntities())
            {
                context.Events.Add(eventToAdd);
                context.SaveChanges();
            }
        }
    }

Questions:

• Would this be a good-enough way to log what I need and is it safe in a multi-user environment?
• How would you do it if you didn't want to dig into logging frameworks?

Answer 1

No, its not. EF6 is not thread-safe (well, DBContext is not thread-safe, so if you have 1 DBcontext per thread you should be ok), so adding your log event can (ha! will) interfere with another thread writing a different log event.

What you will need to do is synchronise all the calls to EF in your task, what I'd do is add the log to a collection and then have another thread that pulls these logs off the collection and writes them to the DB, one at a time. The add/remove calls to the collection must be protected with a lock to stop 2 threads adding (or removing) an entry simultaneously.

Or I'd use log4net, which is thread-safe, so you can simply call it in your thread to do the writing or the log entries.

My other advice is to write the logs to a file rather than the DB. File-logging is quick so you can do it on the main thread with minimal performance impact (log4net is very efficient too, so just use that - and I know IIS will be writing access and error logs to file anyway) which will remove your issue with threading.

One thing I really do know is that if you're not too hot on multi-threading then you need to stop doing it until you are hot on it. No disrespect to you for trying, but thread bugs can be f***** nightmares to solve, you cannot reproduce them nor easily catch them in a debugger and their cause are often unrelated to the symptoms that are reported.

Answer 2

As mentioned by @gbjbaanb you should implement this as a queue with one or more threads actually doing the database work via EF. You can make use of BlockingCollection<T> backed by a ConcurrentQueue<T> to do this in a producer/consumer fashion. Basic premise is each caller logging something simply adds to the queue (i.e. producer). You can then have one or more threads pulling information off the queue and persisting to the database (i.e. consumer). You'd need a separate context for each thread.

There's a reasonable example on the BlockingCollection<T> documentation pages.

Answer 3

Yes, it is safe in a multithread environment. Because you are creating a new instance of the database context everytime you insert an event into the database, you are safe. EF is not thread-safe only if you would try to reuse the same context instance across threads.

The only possible issue is that doing this in an async way possibly means that multiple connections are opened concurrently and the connection pool could be depleted. The more often you log, the higher is the possibility that this happens.

Having said this, I still recommend you use log4net or any existing logging infrastructure and just find a way to log asynchronously. People blog often how to do this with log4net, take a look here for example

http://www.ben-morris.com/using-asynchronous-log4net-appenders-for-high-performance-logging

Note that issues related to async logging are also discussed there. To workaround some issues like the possibility of misordering entries, you could possibly have a queue in such custom appender. Sticking with existing framework lets you reuse a lot of ideas that are already developed. Going on your own sooner or later will stop you for longer, when requirements change.