7

I have a need for users to be able to scan a series of items and for each item print off x number of labels. I am currently trying to use a background worker to accomplish this but I have run into a problem where they are scanning items so fast and there are so many labels to print for each item the background worker chokes. This is how I am generating the background worker thread for each scan because contention was occurring when there was a large number of labels be printed.

 private void RunPrintWorker()
    {
        if (printWorker.IsBusy)
        {
            printWorker = new BackgroundWorker();
            printWorker.DoWork += new DoWorkEventHandler(printWorker_DoWork);
            printWorker.RunWorkerAsync();
        }
        else
            printWorker.RunWorkerAsync();
    }

I don't get any exceptions from the background worker it just seems to not be creating threads fast enough. I am newer to using multiple threads so can anyone point me in the direction of what I am doing wrong?

Thanks.

EDIT: Thanks everyone for the suggestions and reading material this should really help. The order the labels are being printed doesn't really matter since they are scanning pretty fast and the labels are only being printed to one printer too. I will mark an answer after I get the implementation up and running.

EDIT: Austin, below is how I have my printing method setup. Before I was just calling LabelPrinter.PrintLabels in my RunPrintWorker method. Now that I am redoing this I can't figure out what to pass into the SizeQueue method. Should I be passing the newly created print document into it?

 public class LabelPrinter
{
    private int CurrentCount = 0;

    private List<int> _selectedRows = new List<int>();
    public List<int> SelectedRows
    {
        get { return _selectedRows; }
        set { _selectedRows = value; }
    }

    private string _selectedTemplate;
    public string SelectedTemplate
    {
        get { return _selectedTemplate; }
        set { _selectedTemplate = value; }
    }

    private string _templateDirectory = string.Empty;
    public string TemplateDirectory
    {
        get { return _templateDirectory; }
        set { _templateDirectory = value; }
    }

    public void PrintLabels(PrintDocument printDoc, PageSettings pgSettings, PrinterSettings printerSettings, List<int> selectedRows, string selectedTemplate, string templateDir)
    {
        this._selectedRows = selectedRows;
        this._selectedTemplate = selectedTemplate;
        this._templateDirectory = templateDir;

        printDoc.DefaultPageSettings = pgSettings;
        printDoc.PrinterSettings = printerSettings;

        printDoc.PrinterSettings.MaximumPage = selectedRows.Count();
        printDoc.DefaultPageSettings.PrinterSettings.ToPage = selectedRows.Count();
        printDoc.PrinterSettings.FromPage = 1;

        printDoc.PrintPage += new PrintPageEventHandler(printDoc_PrintPage);

        printDoc.Print();
    }

    private void printDoc_PrintPage(object sender, PrintPageEventArgs e)
    {
        CurrentCount = DrawLabel.DrawLabelsForPrinting(e, SelectedTemplate, SelectedRows, CurrentCount, TemplateDirectory);
    }
}
Nathan
  • 5,059
  • 16
  • 48
  • 61
  • 7
    I've found BackgroundWorkers to be adequate only for firing off one or two threads (I've also seen overlap where one process would step on another). You may want to try using a queuing system rather than firing off a thread each time a print request is made. Then, have one BackgroundWorker grab each item from the queue, one at a time until exhausted. – Michael Todd Feb 25 '10 at 22:32
  • That should be an answer. It's a very good suggestion. – David Feb 25 '10 at 22:34

3 Answers3

8

Try adding the items to a queue (for example, Queue<Item>) and have the BackgroundWorker process the queue.

EDIT: Adding some simple, untested code that may work for you. I would encapsulate the print queue with its processor and just send it jobs.

class SimpleLabelPrinter
{
    public bool KeepProcessing { get; set; }
    public IPrinter Printer { get; set; }

    public SimpleLabelPrinter(IPrinter printer)
    {
        Printer = printer;
    }


    /* For thread-safety use the SizeQueue from Marc Gravell (SO #5030228) */        
    SizeQueue<string> printQueue = new SizeQueue<string>();

    public void AddPrintItem(string item)
    {
        printQueue.Enqueue(item);
    }

    public void ProcessQueue()
    {
        KeepProcessing = true;

        while (KeepProcessing)
        {
            while (printQueue.Count > 0)
            {
                Printer.Print(printQueue.Dequeue());
            }

            Thread.CurrentThread.Join(2 * 1000); //2 secs
        }
    }
}

class Program
{
    static void Main(string[] args)
    {
        SimpleLabelPrinter printer1 = new SimpleLabelPrinter(...);
        SimpleLabelPrinter printer2 = new SimpleLabelPrinter(...);

        Thread printer1Thread = new Thread(printer1.ProcessQueue);
        Thread printer2Thread = new Thread(printer2.ProcessQueue);

        //...

        printer1.KeepProcessing = false;  //let the thread run its course...
        printer2.KeepProcessing = false;  //let the thread run its course...
    }
}

SizeQueue implementation

EDIT 2: Addressing the updated code in the question

First, I would define a PrintJob class that contains the number of copies to print and either the complete label text or enough data to derive it (like IDs for a DB query). This would lead you to replace SizeQueue<string> in my code above to SizeQueue<PrintJob> as well as AddPrintItem(string item) to AddPrintJob(PrintJob job).

Second, I would keep your LabelPrinter code separated (perhaps create that IPrinter interface) and pass that into the constructor of my SimpleLabelPrinter (which may not be the best name at this point but I'll let you handle that).

Next, create your LabelPrinter and SimpleLabelPrinter (say printer1 for this example) wherever it's appropriate for your app (in your apps Closing or "cleanup" method, be sure to set KeepProcessing to false so its thread ends).

Now when you scan an item you send it to the SimpleLabelPrinter as:

printer1.AddPrintJob(new PrintJob(labelText, numberOfCopies));
Community
  • 1
  • 1
Austin Salonen
  • 49,173
  • 15
  • 109
  • 139
  • Beat me to that. However having multiple threads might be a good idea. – AxelEckenberger Feb 25 '10 at 22:36
  • 2
    I've built a system for this and multiple *printing* threads may cause more problems than they solve -- end-users expecting printouts in the scanned order, for example. If you're printing to multiple printers then multiple printing threads makes sense. – Austin Salonen Feb 25 '10 at 22:39
  • 3
    only if he has more than one printer :) – stmax Feb 25 '10 at 22:39
  • 1
    Don't forget the Queue class isn't thread safe. You need to lock/synchronise any en-queue actions. (Although you can have multiple simultaneous readers). – Simon P Stevens Feb 25 '10 at 23:02
  • I'm sorry but I'm going to have to downvote because of the code sample you have added. That code is not thread safe. You are not taking any locks on the queue. the MSDN docs for the queue class (http://msdn.microsoft.com/en-us/library/7977ey2c.aspx) state that enqueue operations must be synchronised. I'll be happy to revoke my downvote if you fix or remove the code sample. – Simon P Stevens Feb 25 '10 at 23:12
  • @Austin: Good edit, nice reference, down vote removed and +1 for responding to comments and producing a nice sample. – Simon P Stevens Feb 26 '10 at 08:45
  • @Austin. It wouldn't let me +1, so it'll have to be a virtual +1 but I've removed the down vote. – Simon P Stevens Feb 26 '10 at 08:46
  • @Nathan: Use separate classes. You could probably inline your `Printer` code but keep your version of `SimpleLabelPrinter` and `SizeQueue` as separate classes. – Austin Salonen Feb 26 '10 at 15:15
  • Where do you get IPrinter from? I can't find this interface. – Nathan Feb 26 '10 at 15:19
  • @Nathan: I made it up. I've created a similar interface in some recent work to support multiple label printing implementations. – Austin Salonen Feb 26 '10 at 15:48
  • Oh ok. That makes sense then. Austin please see my new edit back in the main question. – Nathan Feb 26 '10 at 16:06
  • Sorry one more thing, while (printQueue.Count > 0) is underlined because there is no Count for printQueue. Do I need to implement IList to solve this issue? – Nathan Feb 26 '10 at 20:57
  • @Nathan: You'll have to expose that in the SizeQueue implementation. – Austin Salonen Feb 26 '10 at 21:13
7

Basically what you are doing is saying if the printer is busy, overwrite your printWorker object with another worker and start that one. You then have no reference to your old worker object and don't do any cleanup.

There are all kinds of problems with this.

  • Background workers must be disposed of when finished to prevent leaks.
  • Creating more background works doesn't mean the printer will run any faster, it just means you have more workers trying to get to the printer at the same time.

What you need to do is think about queuing. I would say you have two main options.

First - Use ThreadPool.QueueUserWorkItem(...)

ThreadPool.QueueUserWorkItem(new WaitCallback(o=>{printWorker_DoWork();}));

This will use the .net threadpool to queue up your tasks and process them on the pool. (The pool will auto size to an appropriate number of threads to handle your queued requests). This requires no disposal or clean up, you just fire and forget, although you need to be aware that the work items are not guaranteed to be processed in the same order you add them in (it's likely some will be processed simultaneously), so if you care about order, this is no good.

Second - Implement the Producer-consumer thread queue pattern. This requires a synchronised queue to which one thread, or threads (the producer) adds items, and other threads (the consumers) remove and process those items. If you are new to threading this can be quite tricky to get right as you have to make sure your reading/writing to the shared queue is properly synchronised so the order is maintained and nothing is lost.

Be careful of the Queue<T> type, although it could be used, it is not automatically thread safe on it's own. Make sure if you do use it you add some of your own locking and synchronisation.

From the MSDN page:

Any instance members are not guaranteed to be thread safe.

A Queue<T>) can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. To guarantee thread safety during enumeration, you can lock the collection during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

I would recommend the more simple first option if you don't care about order, but if you find you need the more fine grained control over the queuing and processing, or order is important, that is when you could move to the more complex pattern - if you follow the links at the bottom of the article, you will find examples and source code, alternatively you can just use the Queue class and make sure you get your locking right.

Community
  • 1
  • 1
Simon P Stevens
  • 27,303
  • 5
  • 81
  • 107
  • +1 for the not-cleaninng-up issue. But the Bgw uses the ThreadPool too, so QueueUserWorkItem is not really different. – H H Feb 25 '10 at 22:52
  • @Henk, Thanks. the fact it doesn't require cleanup, is enough difference in this case to warrant the change I think. I'd say the bgw is designed as a class that is for performing a single action in the background. It's not really designed for scenarios involving multiple thread creation in the way Nathan was using it. – Simon P Stevens Feb 25 '10 at 23:00
  • You can easily have multiple Bgw's, for instance if you want to use ProgressChanged and Completed events. – H H Feb 26 '10 at 09:21
  • @Henk, yeah you can, but each bgw should be assigned a single action, that's what I mean. Creating multiple ones dynamically (although possible) is surely not a great pattern because you have to clean them up, which means you have to hold references to all of them. – Simon P Stevens Feb 26 '10 at 10:52
0

You need to implement a Queuing system. Take a look at the Queue class, and queue up your PrintDocuments in it. Spin up a background thread, (not a backgroundworker in this situation) and have it periodically check the Queue for for more items and print them if it can.

Run a method like this in the backgroundworker:

private void PrintLoop()
{
    while (true)
    {
        if (documents.Count > 0)
        {
            PrintDocument document = documents.Dequeue();
            document.Print();

        }
        else
        {
            Thread.Sleep(1000);
        }
    }
}

Make one BackgroundWorker per printer, and have each print to a different printer.

David Morton
  • 16,338
  • 3
  • 63
  • 73