1

I have a windows form program (**VS 2010 .NET 4 **) that detecting recursively folders and sub folders of a directory and optimizing files. I do it by tasking library and have a progressbar that shows progress of item and label near progressbar that shows current file. I have a SetText(string text) method and delegate (delegate void SetTextCallback(string text);) which use for this purpose. when I want to show a messagebox at the end of process I think it deadlock. but when I don't use task.Wait(); in button6_Click it goes ok and UI won't hang. this is my code:

    public partial class Form1 : Form
    {


int MaxFileCounter = 0;
    static int FileCounter = 0;

delegate void SetTextCallback(string text);
delegate void SetProgressCallback(int i);

private void button6_Click(object sender, EventArgs e)
{
    Task task = Task.Factory.StartNew(() =>
    {
        editFiles(txtFilePath.Text);
    });

    task.Wait();

    MessageBox.Show("finished");
}

private void editFiles(string directoryPath)
{
    try
    {
        //DirectoryInfo dirInfo = new DirectoryInfo(txtFilePath.Text);
        DirectoryInfo dirInfo = new DirectoryInfo(directoryPath);
        //string[] extensionArray = { ".jpg", ".png", ".gif" ,".jpeg"};
        string[] extensionArray = txtExtensionList.Text.Split(';');
        HashSet<string> allowedExtensions = new HashSet<string>(extensionArray, StringComparer.OrdinalIgnoreCase);

        FileInfo[] files = Array.FindAll(dirInfo.GetFiles(), f => allowedExtensions.Contains(f.Extension));
        writeFiles(files);

        foreach (DirectoryInfo dir in dirInfo.GetDirectories())
        {
            editFiles(dir.FullName);
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}

private void writeFiles(FileInfo[] files)
{
    try
    {
        foreach (FileInfo fileinfo in files)
        {
            MemoryStream mo;

            using (Image image = Image.FromFile(fileinfo.FullName))
            {

                SetText(fileinfo.FullName);

                FileCounter++;

                SetProgress(FileCounter * 100 / MaxFileCounter);

                mo = (MemoryStream)optimizeImage(image, int.Parse(txtPercent.Text));
            }

            byte[] bt = new byte[mo.Length];
            mo.Read(bt, 0, bt.Length);
            mo.Flush();
            mo.Close();

            string fullpath = fileinfo.FullName;

            File.WriteAllBytes(fullpath, bt);

        }
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}

private void SetText(string text)
{
    // InvokeRequired required compares the thread ID of the
    // calling thread to the thread ID of the creating thread.
    // If these threads are different, it returns true.
    if (lblFileName.InvokeRequired)
    {
        SetTextCallback d = new SetTextCallback(SetText);
        this.Invoke(d, new object[] { text });
    }
    else
    {
        this.lblFileName.Text = text;
    }
}
private void SetProgress(int i)
{
    if (progressBar1.InvokeRequired)
    {
        SetProgressCallback p = new SetProgressCallback(SetProgress);
        this.Invoke(p, new object[] { i });
    }
    else
    {
        this.progressBar1.Value = i;
    }
}
}

how can I handle it?

Andy G
  • 19,232
  • 5
  • 47
  • 69
Reza Akraminejad
  • 1,412
  • 3
  • 24
  • 38
  • "how can I handle it?" - remove the `task.Wait();`. It is doing the wrong thing at the wrong moment. – H H Jun 07 '14 at 13:04
  • I know if I remove it, it will work.so when should I announce user when the job finished? when lots of files are being proccessed? – Reza Akraminejad Jun 07 '14 at 13:13
  • @Hamed_gibago There is no difference between `starting a task and wait for it to finish` and `running the code(editFiles) in the UI thread`. In both ways, UI will be unresponsive. – L.B Jun 07 '14 at 13:15
  • 1
    Make EditFiles aware of its recursion depth and raise an event when the top level is finished. – H H Jun 07 '14 at 13:18
  • How should I do that? – Reza Akraminejad Jun 07 '14 at 13:21
  • [Dispatching an event](http://stackoverflow.com/a/2448530/60761). And then make the hanlding thread-safe, you already know how to do that. – H H Jun 07 '14 at 13:30
  • Or use a backgroundWorker, it has a Completed event, Progress and Exception handling. – H H Jun 07 '14 at 13:33

1 Answers1

3
    this.Invoke(d, new object[] { text });

The point of using a Task is to not wait for it. If you do anyway then you'll hang the UI thread. It goes catatonic, no longer being responsive to notifications from Windows. Including the ones that your own code generates, like the Invoke() call does. Which sends a message to the UI thread to go look for the delegate to invoke.

So the Invoke() call cannot complete, the UI thread is hung. The Task.Wait() call cannot complete since the task is hung on the Invoke() call. A deadly embrace, one of the standard threading bugs called deadlock.

Note how BeginInvoke() doesn't have this problem, it doesn't wait for completion. Solves the deadlock, doesn't provide you with a live update since you still have a UI thread that's dead to the world. You must remove the Wait() call. You can get the MessageBox.Show() call to run by adding a task continuation with TaskScheduler.FromCurrentSynchronizationContext(). And you need to fret about getting this task to stop when the user closes the window, letting it continue running will not (in general) turn out well. The async/await keywords added to C# version 5 are a more general solution to this problem.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • I add a condition if (FileCounter == MaxFileCounter) MessageBox.Show("Finished"); after File.WriteAllBytes(fullpath, bt); in writeFiles method. that check if job finished and all files traced. and removed the task.wait() in button6_Click . it works. but not the threading way solved. is that a good solving? I accepted your answer.that's ok. and I will test that. but talk about my solution. good or bad? – Reza Akraminejad Jun 08 '14 at 05:11