CODEWITHSUNDEEP
c#multithreadingqueue
tradetree
tradetree

Reputation: 438

C# Enqueue Failure

I have a simple logging mechanism that should be thread safe. It works most of the time, but every now and then I get an exception on this line, "_logQ.Enqueue(s);" that the queue is not long enough. Looking in the debugger there are sometimes just hundreds of items, so I can't see it being resources. The queue is supposed to expand as needed. If I catch the exception as opposed to letting the debugger pause at the exception I see the same error. Is there something not thread safe here? I don't even know how to start debugging this.

    static void ProcessLogQ(object state)
    {
        try
        {
            while (_logQ.Count > 0)
            {
                var s = _logQ.Dequeue();
                string dir="";
                Type t=Type.GetType("Mono.Runtime");
                if (t!=null)
                {
                    dir ="/var/log";
                }else
                {
                    dir = @"c:\log";
                    if (!Directory.Exists(dir))
                        Directory.CreateDirectory(dir);
                }
                if (Directory.Exists(dir))
                {
                    File.AppendAllText(Path.Combine(dir, "admin.log"), DateTime.Now.ToString("hh:mm:ss ") + s + Environment.NewLine);
                }
            }
        }
        catch (Exception)
        {
        }
        finally
        {
            _isProcessingLogQ = false;
        }
    }

    public static void Log(string s) {
        if (_logQ == null)
            _logQ = new Queue<string> { };

        lock (_logQ)
            _logQ.Enqueue(s);
        if (!_isProcessingLogQ) {
            _isProcessingLogQ = true;
            ThreadPool.QueueUserWorkItem(ProcessLogQ);
        }
    }

Note that the threads all call Log(string s). ProcessLogQ is private to the logger class.

* Edit * I made a mistake in not mentioning that this is in a .NET 3.5 environment, therefore I can't use Task or ConcurrentQueue. I am working on fixes for the current example within .NET 3.5 constraints.

** Edit * I believe I have a thread-safe version for .NET 3.5 listed below. I start the logger thread once from a single thread at program start, so there is only one thread running to log to the file (t is a static Thread):

    static void ProcessLogQ()
    {
        while (true) {
            try {
                lock (_logQ);
                while (_logQ.Count > 0) {
                    var s = _logQ.Dequeue ();
                    string dir = "../../log";
                    if (!Directory.Exists (dir))
                        Directory.CreateDirectory (dir);
                    if (Directory.Exists (dir)) {
                        File.AppendAllText (Path.Combine (dir, "s3ol.log"), DateTime.Now.ToString ("hh:mm:ss ") + s + Environment.NewLine);
                    }
                }
            } catch (Exception ex) {
                Console.WriteLine (ex.Message);
            } finally {
            }
            Thread.Sleep (1000);
        }
    }

    public static void startLogger(){
        lock (t) {
            if (t.ThreadState != ThreadState.Running)
                t.Start ();
        }
    }
    private static void multiThreadLog(string msg){
        lock (_logQ)
            _logQ.Enqueue(msg);
    }

Upvotes: 0

Views: 1272

Answers (2)

Samir Banjanovic
Samir Banjanovic

Reputation: 400

Look at the TaskParallel Library. All the hard work is already done for you. If you're doing this to learn about multithreading read up on locking techniques and pros and cons of each.

Further, you're checking if _logQ is null outside your lock statement, from what I can deduce it's a static field that you're not initializing inside a static constructor. You can avoid doing this null check (which should be inside a lock, it's critical code!) you can ensure thread-safety by making it a static readonly and initializing it inside the static constructor.

Further, you're not properly handling queue states. Since there's no lock during the check of the queue count it could vary on every iteration. You're missing a lock as your dequeuing items.

Excellent resource: http://www.yoda.arachsys.com/csharp/threads/

Upvotes: 1

Rufus L
Rufus L

Reputation: 37020

For a thread-safe queue, you should use the ConcurrentQueue instead:

https://msdn.microsoft.com/en-us/library/dd267265(v=vs.110).aspx

Upvotes: 1

Related Questions