kovac
kovac

Reputation: 5389

Why is Thread.Sleep() used in following context and how to avoid it?

I'm going through following method which is sending messages over Http.

        private static void HTTPProcessQueue()
        {
            while (true)
            {
                try
                {
                    Thread.Sleep(10000);
                    Utils.LogDebug("Msg Queue Check");
                    while (msgQueue.Count > 0)
                    {
                        QueueItem queueItem;
                        lock (msgQueue)
                        {
                            queueItem = msgQueue.Dequeue();
                        }
                        if (queueItem != null)
                            if(!HTTPTransmitEmailItem(queueItem.username, queueItem.filename))
                                Thread.Sleep(5000);
                    }
                }
                catch (Exception ex)
                {
                }
            }
        }
  1. In the code above, why are Thread.Sleep(10000) and Thread.Sleep(5000) used in lines 7 and 18?
  2. Also, why is there a while(true) in line 3?

Upvotes: 0

Views: 199

Answers (2)

ProgrammingLlama
ProgrammingLlama

Reputation: 38727

As you requested, here is a slightly better way of doing it:

private static System.Collections.Concurrent.BlockingCollection<MsgType> msgQueue = new System.Collections.Concurrent.BlockingCollection<MsgType>();

private static void AddQueueItems() // simulate adding items to the queue
{
    msgQueue.Add(new MsgType());
    msgQueue.Add(new MsgType());
    msgQueue.Add(new MsgType());
    msgQueue.Add(new MsgType());

    // when adding is done, or the program is shutting down
    msgQueue.CompleteAdding();
}

private static void HTTPProcessQueue()
{
    foreach (var queueItem in msgQueue.GetConsumingEnumerable())
    {
        if (queueItem != null)
        {
            if (!HTTPTransmitEmailItem(queueItem.username, queueItem.filename))
            {
                Thread.Sleep(5000);
            }
        }
    }
}

I'd recommending using the async/await pattern with HTTPTransmitEmailItem, and then you can use await Task.Delay(...) instead of Thread.Sleep(...). I've also not included any error checking in this code.

This would then look more like:

private static async Task HTTPProcessQueue()
{
    foreach (var queueItem in msgQueue.GetConsumingEnumerable())
    {
        if (queueItem != null)
        {
            if (!(await HTTPTransmitEmailItemAsync(queueItem.username, queueItem.filename)))
            {
                await Task.Delay(5000);
            }
        }
    }
}

But you would have to make a HttpTransmitEmailItemAsync method. Also note that the GetConsumingEnumerable(...) method has an overload which takes a CancellationToken, so you could use this to gain more control over when to end the queue process. You can learn about async/await here.

Upvotes: 2

mdsuffian
mdsuffian

Reputation: 33

  1. The Thread.Sleep(10000) is used on line 7 to let the system pause / wait for 10 seconds before it starts the function Utils.LogDebug("Msg Queue Check"); to log the debug information with message "Msg Queue Check". and i believe the Thread.Sleep(5000) is added at the end of loop to create a delay or to wait for 5 seconds before process the next loop.

  2. while(true) is usually used for infinite loop. all method inside this loop will run in loop in infinite time.

Upvotes: 0

Related Questions