3

I'm using a background worker to handle the loading of a file to stop my ui from freezing however it seems that the RunWorkerCompleted is finishing before my DoWork event has completed (Causes errors when exiting dialog)... is there anything I'm doing wrong? Am I better off doing this over a task?

public static <T> LoadDesign(string xmlPath)
{
    PleaseWait pw = new PleaseWait(xmlPath);
    pw.ShowDialog();
    return pw.design;
}


private PleaseWait(string xmlFile)
{
   InitializeComponent();
   bw = new BackgroundWorker();
   bw.WorkerSupportsCancellation = true;
   bw.DoWork += (s, e) =>
   {
      design = (Cast)DllCall containing XmlSerializer.Deserialize(...,xmlFile);
   };
   bw.RunWorkerCompleted += (s, e) => {
   //Exit please wait dialog
      this.Close(); 
   };
   if (!bw.IsBusy)
       bw.RunWorkerAsync();
}

I believe the issue may be down to the fact that my background worker is calling a dll and not waiting for the response. I've tried to add checks such as while(design == null) to no avail..

Edit2 The error is NRE as the design hasn't been loaded, I can easily fix this but would rather get the threading working instead.

Sayse
  • 42,633
  • 14
  • 77
  • 146
  • If you want an answer then provide full details about the core code. This `(cast) DllCall` is too much pseudo code. – H H May 29 '13 at 07:53
  • @HenkHolterman - I've added the method but I don't see how relevant it is? (also in process of adding `Using to dll`) – Sayse May 29 '13 at 07:57
  • What error do you see ? – YK1 May 29 '13 at 08:00
  • I'm more interested in how exactly you call it. It's the only relevant thing inside DoWork. – H H May 29 '13 at 08:02
  • 1
    Is there anything in `DoWork` or the external call that is asynchronous? Nothing that you've shown appears to be. Otherwise I can't see how "my background worker is calling a dll and not waiting for the response" makes any sense. – Mike Zboray May 29 '13 at 08:02
  • I'm struggling to see what its doing also, mike z. @HenkHolterman - Its just a static method that returns an object? Basically `DLL.Deserialize()` – Sayse May 29 '13 at 08:15
  • Then fix the code and add any clarification as a comment. This is the cruciasl part... – H H May 29 '13 at 08:23
  • What about an exception in the DoWork method? It then also fires the RunWorkerCompleted event where the event args will provide information about the reason and the possible error. – Youp Bernoulli May 29 '13 at 08:39
  • No errors, through break points set; one in dll and one in the RunWorkerCompleted, the break point in the worker completed is hit before stepping through past writer.Deserialize – Sayse May 29 '13 at 08:52

3 Answers3

7

There are lots of little mistakes. Given that we are probably not looking at the real code and that we don't have a debugger with a Call Stack window to see where it actually crashes, any one of them might be a factor.

  • Testing bw.IsBusy and not starting the worker when it is true is a grave mistake. It can't ever be busy in the code as posted but if it actually is possible for it to be true then you've got a nasty bug in your code. Since you actually did subscribe the events on a busy worker. Now the RunWorkerCompleted event handler will run twice.

  • Using the Close() method to close a dialog is not correct. A dialog should be closed by assigning its DialogResult property. Not the gravest kind of mistake but wrong nonetheless.

  • There's a race in the code, the worker can complete before the dialog is ever displayed. A dialog can only be closed when its native window was created. In other words, the IsHandleCreated must be true. You must interlock this to ensure this can never happen. Subscribe the dialog's Load event to get the worker started.

  • You blindly assume that the worker will finish the job and produce a result. That won't be the case when its DoWork method died from an exception. Which is caught by BackgroundWorker and passed to the RunWorkerCompleted event handler as the e.Error property. You must check this property and do something reasonable if it isn't null.

Judging from the comments, I'd guess at the latter bullet being the cause. You debug this by using Debug + Exceptions, tick the Thrown checkbox for CLR exceptions. The debugger will now stop when the exception is thrown, allowing you to find out what went wrong.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Thank You Hans, you were indeed correct about my blind assumptions. I would have expected this error to have been thrown though? Also, do you have any reference as to why I should use DialogResult? Im intrigued... – Sayse May 29 '13 at 11:09
  • The exception is certainly thrown. And caught, so it can be passed to the RunWorkerCompleted event handler. This was intentional design, BGW wouldn't be that popular if it was liable to crash programs frequently. I'd recommend any book about Winforms programming to learn more about dialogs. There are plenty of good ones available. Or just click the Ask Question button. – Hans Passant May 29 '13 at 11:40
2

If you Call another async function in your BackgroundWorker _DoWork event,

like;

    private void BackgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
    {
        somethingsToDoAsync();
        // somethingsToDoAsync() function is to ASYNC 
    }

_RunWorkerCompleted fires even before completed _Dowork event.

Change other somethingsToDoAsync() function to not async.

Cihangir Ozdemir
  • 21
  • 2
1

It maybe possible your background worker actually does not take much time and complete before the dialog is shown. I'd suggest shift the background worker initialization and start up code to PleaseWait's Form_Load or Form_Shown

YK1
  • 7,327
  • 1
  • 21
  • 28
  • 1
    I'd even shift it to `Shown`, as this is the event fired when the form is displayed. `Load` may be called far before `Shown`. – Thorsten Dittmar May 29 '13 at 08:18
  • I appologise YK1 the error wasn't to do with the form.Close but I can understand why it looked like this and thank you for your answer.. I'll update question – Sayse May 29 '13 at 08:34