c# Thread which creates image getting cancelled

Question

I have a event handler attached to the selectionChanged event on a DataGridView. In this handler I need to create and load an image and then display it in a picture box. The trouble I'm having is, if I jump between row selections quickly the application seems to hang, which is the issue I was trying to avoid.

Here is my code:

    private void loadJobSheet(Job currentJob)
    {

        if (this.jobCardImageThread != null && this.jobCardImageThread.IsAlive)
            this.jobCardImageThread.Abort();

        Image jobCardImage = null;
        this.jobCardImageThread = new Thread(new ThreadStart(
            delegate()
            {
                SavedDocument document = currentJob.SavedDocument;
                DocumentConverter<Bitmap> converter = DocumentConverterFactory<Bitmap>.getDocumentConverterForType(Path.GetExtension(document.Document_Name).Replace('.', ' ').Trim().ToUpper(), typeof(Bitmap));
                jobCardImage = (Image)converter.convertDocument(FileUtils.createTempFile(document.Document_DocumentData.ToArray(), document.Document_Name));
            }
        ));

        jobCardImageThread.Start();
        this.picLoadingJobCard.Visible = true;

        jobCardImageThread.Join();

        if (jobCardImage != null)
        {
            this.picJobCard.Image = jobCardImage;
            this.picLoadingJobCard.Visible = false;
        }
    }

score 0 · Answer 1 · answered Sep 17 '13 at 09:33

You are waiting for the separate thread to finish when you do

jobCardImageThread.Join();

This blocks the UI thread, which suspends the application.

You should remove the Join() call, create a separate method out of anything after the Join() call, and call that method from the delegate. Probably use an Invoke(...) call to switch back to the UI thread.

Uwe · Answer 2 · 2013-09-17T11:08:22.060

I think your problem is jobCardImageThread.Join(); With this statement you tell your Thread to wait for the other to finish. This way your UI is hanging.

Why don't you use a background-worker. For example:

Put this into your constructor

        this.backgroundWorker = new BackgroundWorker();
        this.backgroundWorker.DoWork += new DoWorkEventHandler(backgroundWorker_DoWork);
        this.backgroundWorker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(backgroundWorker_RunWorkerCompleted);
        this.backgroundWorker.WorkerSupportsCancellation = true;

And add the following methods:

    private BackgroundWorker backgroundWorker;
    private AutoResetEvent resetEvent = new AutoResetEvent(false);
    private Thread thread;

    private void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
    {
        this.picLoadingJobCard.Visible = true;
        Job currentJob = (Job)e.Argument;
        SavedDocument document = currentJob.SavedDocument;
        DocumentConverter<Bitmap> converter = DocumentConverterFactory<Bitmap>.getDocumentConverterForType(Path.GetExtension(document.Document_Name).Replace('.', ' ').Trim().ToUpper(), typeof(Bitmap));
        Image jobCardImage = (Image)converter.convertDocument(FileUtils.createTempFile(document.Document_DocumentData.ToArray(), document.Document_Name));

        e.Result = jobCardImage;
    }

    private void backgroundWorker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
    {
        if (e.Error != null)
        {
            //error-handling
        }
        else if (e.Cancelled)
        {
            //cancel-handling  
        }
        else
        {
            Image jobCardImage = e.Result as Image;
            if (jobCardImage != null)
                this.picJobCard.Image = jobCardImage;
        }

        this.picLoadingJobCard.Visible = false;
        this.resetEvent.Set();
    }


    private void loadJobSheet(Job currentJob)
    {
        if (this.thread != null)
            this.thread.Abort();

        this.thread = new Thread(new ThreadStart(
        delegate()
        {
            if (this.backgroundWorker.IsBusy)
            {
                this.backgroundWorker.CancelAsync();
                this.resetEvent.WaitOne();
            }
            this.backgroundWorker.RunWorkerAsync(currentJob);
        }));
        this.thread.Start();
    }

The concept is generally ok, but it won't work properly if a new job is added before the first one has finished. `CancelAsync()` does nothing much except setting the [`CancellationPending`](http://msdn.microsoft.com/en-us/library/system.componentmodel.backgroundworker.cancellationpending.aspx) property to `true`. You should revise your code to add appropriate synchronization to your method, because `RunWorkerAsync` will throw an exception if the worker is still running. — vgru, Sep 17 '13 at 09:45
You're absolutely right. This time it should be correct (I hope :-) — Uwe, Sep 17 '13 at 10:15
Now the problem is that `loadJobSheet` creates a new thread on each call, meaning that you can have a race condition where multiple threads block on `WaitOne` and them simultaneously call `RunWorkerAsync`. — vgru, Sep 17 '13 at 10:39

score 0 · Answer 3 · edited May 23 '17 at 12:11

If you create a background Thread and immediately call Join after running it, you basically just wasted time and memory for creating a synchronous method, because your current thread will block until the background thread is finished. If the current thread is a UI thread, this will be pretty obvious.

Also, using Thread.Abort to kill a thread is not recommended.

I would suggest creating a long-lived background thread which will most of the time wait for a signal from the main thread. This will ensure that you don't unnecessarily create multiple threads in case you are receiving more requests than your worker method can handle.

This is the general idea:

// have a long lived and prosperous thread which handles jobs
private readonly Thread _backgroundWorker;

// you need a way to signal the thread to continue running
private readonly AutoResetEvent _signalNewTask;

// you need a flag indicating you want to stop (to avoid aborting the thread)
private volatile bool _keepRunning;

// and you need to pass the current job to that thread
private volatile Job _currentJob;

The loop should look something like this:

// this runs on a background thread
private void WorkerLoop()
{
    Job lastJob = null; Image lastResult = null;

    while (_keepRunning)
    {
        // use an AutoResetEvent for cross-thread signalization
        _signalNewTask.WaitOne();

        // make sure the app isn't ending
        if (!_keepRunning)
            break;

        // don't bother if we already processed this job
        if (lastJob == _currentJob)
            continue;

        // capture the job in a local variable
        lastJob = _currentJob;

        // long processing
        lastResult = LoadImage(lastJob);

        // check if this is still the last requested job
        if (_keepRunning && lastJob == _currentJob)
            DisplayImage(lastResult);
    }
}

To schedule a job for execution, you simply set the field and signal the event:

private void ScheduleNewJob(Job nextJob)
{
    // don't waste time if not needed
    if (nextJob == _currentJob)
        return;

    _picLoadingJobCard.Visible = true;
    _currentJob = nextJob;
    _signalNewTask.Set();
}

You'll also need to add initialization and cleanup code to your Form:

public SomeForm()
{
    InitializeComponent();

    _keepRunning = true;
    _signalNewTask = new AutoResetEvent(false);
    _backgroundWorker = new Thread(WorkerLoop);
    _backgroundWorker.IsBackground = true;
    _backgroundWorker.Priority = ThreadPriority.BelowNormal;
    _backgroundWorker.Start();
}

protected override void OnFormClosed(FormClosedEventArgs e)
{
    // set the running flag to false and signal the thread 
    // to wake it up
    _keepRunning = false;
    _signalNewTask.Set();

    // this will lock very shortly because the background
    // thread breaks when the flag is set
    _backgroundWorker.Join();

    base.OnFormClosed(e);
}

And since DisplayImage (or whatever) will be called from a background thread, you have to schedule on the UI thread by calling Invoke:

private void DisplayImage(Image result)
{
    if (this.InvokeRequired)
    {
        Invoke(new Action<Image>(DisplayImage), result);
        return;
    }

    _picLoadingJobCard.Visible = false;
    _picJobCard.Image = result;
}

c# Thread which creates image getting cancelled

3 Answers3