views:

1262

answers:

5

I have spent the whole day trying to make my application use threads but with no luck. I have read much documentation about it and I still get lots of errors, so I hope you can help me.

I have one big time consuming method which calls the database and updates the GUI. This has to happen all the time(or about every 30 seconds).

public class UpdateController
{
    private UserController _userController;

    public UpdateController(LoginController loginController, UserController userController)
    {
        _userController = userController;
        loginController.LoginEvent += Update;
    }

    public void Update()
    {
        BackgroundWorker backgroundWorker = new BackgroundWorker();
        while(true)
        {
            backgroundWorker.DoWork += new DoWorkEventHandler(backgroundWorker_DoWork);
            backgroundWorker.RunWorkerAsync();
        }     
    }

    public void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
    {
        _userController.UpdateUsersOnMap();
    }
}

With this approach I get an exception because the backgroundworker is not and STA thread(but from what I can understand this is what I should use). I have tried with a STA thread and that gave other errors.

I think the problem is because I try to update the GUI while doing the database call(in the background thread). I should only be doing the database call and then somehow it should switch back to the main thread. After the main thread has executed it should go back to the background thread and so on. But I can't see how to do that.

The application should update the GUI right after the database call. Firering events don't seem to work. The backgroundthread just enters them.

EDIT:

Some really great answers :) This is the new code:

public class UpdateController{
private UserController _userController;
private BackgroundWorker _backgroundWorker;

public UpdateController(LoginController loginController, UserController userController)
{
    _userController = userController;
    loginController.LoginEvent += Update;
    _backgroundWorker = new BackgroundWorker();
    _backgroundWorker.DoWork += backgroundWorker_DoWork;
    _backgroundWorker.RunWorkerCompleted += backgroundWorker_RunWorkerCompleted;
}

public void _backgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    _userController.UpdateUsersOnMap();
}

public void Update()
{   
    _backgroundWorker.RunWorkerAsync();
}

void backgroundWorker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    //UI update
    System.Threading.Thread.Sleep(10000);
    Update();
}

public void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
{
    // Big database task
}

}

But how can I make this run every 10 second? System.Threading.Thread.Sleep(10000) will just make my GUI freeze and while(true) loop in Update() as suggested gives an exception(Thread too busy).

+6  A: 

You need to declare and configure the BackgroundWorker once - then Invoke the RunWorkerAsync method within your loop...

public class UpdateController
{
    private UserController _userController;
    private BackgroundWorker _backgroundWorker;

    public UpdateController(LoginController loginController, UserController userController)
    {
        _userController = userController;
        loginController.LoginEvent += Update;
        _backgroundWorker = new BackgroundWorker();
        _backgroundWorker.DoWork += new DoWorkEventHandler(backgroundWorker_DoWork);
        _backgroundWorker.OnProgressChanged += new ProgressChangedEventHandler(backgroundWorker_ProgressChanged);
        _backgroundWorker.ReportsProgress = true;
    }

    public void Update()
    {
         _backgroundWorker.RunWorkerAsync();    
    }

    public void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
    {
        while (true)
        {
        // Do the long-duration work here, and optionally
        // send the update back to the UI thread...
        int p = 0;// set your progress if appropriate
        object param = "something"; // use this to pass any additional parameter back to the UI
        _backgroundWorker.ReportProgress(p, param);
        }
    }

    // This event handler updates the UI
    private void backgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
    {
        // Update the UI here
//        _userController.UpdateUsersOnMap();
    }
}
IanR
This throws an exception. "This BackgroundWorker is currently busy and cannot run multiple tasks concurrently." :(
bobjink
sorry - yes it would, sorry! Please see update - I have shifted the while loop into the single instance of the 'DoWork' method; this will allow you to push UI notifications via 'ReportProgress', and then if you like, you can add a method handler for the 'Completed' if you ever want to break out of your infinite loop... cheers :)
IanR
... and you can send this worker thread to Sleep() for 10 seconds without impacting the UI thread!
IanR
This seems to work! Unfortunately I can't see/try it out. All this threading is now the cause of my animations between screens not working. I dont know why :( But thats a whole new problem. Marked as accepted answer. Thanks :)
bobjink
+3  A: 

You have to use the Control.InvokeRequired property to determine if you are on a background thread. Then you need to invoke your logic that modified your UI via the Control.Invoke method to force your UI operations to occur on the main thread. You do this by creating a delegate and passing it to the Control.Invoke method. The catch here is you need some object derived from Control to call these methods.

Edit: As another user posted, if yo you can wait to the BackgroundWorker.Completed event to update your UI then you can subscribe to that event and call your UI code directly. BackgroundWorker_Completed is called on the main app thread. my code assumes you want to do updates during the operation. One alternative to my method is to subscribe to the BwackgroundWorker.ProgressChanged event, but I believe you'll need to still call Invoke to update your UI in that case.

for example

public class UpdateController
{
    private UserController _userController;        
    BackgroundWorker backgroundWorker = new BackgroundWorker();

    public UpdateController(LoginController loginController, UserController userController)
    {
        _userController = userController;
        loginController.LoginEvent += Update;
    }

    public void Update()
    {                        
         // The while loop was unecessary here
         backgroundWorker.DoWork += new DoWorkEventHandler(backgroundWorker_DoWork);
         backgroundWorker.RunWorkerAsync();                 
    }

    public delegate void DoUIWorkHandler();


    public void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
    {
       // You must check here if your are executing on a background thread.
       // UI operations are only allowed on the main application thread
       if (someControlOnMyForm.InvokeRequired)
       {
           // This is how you force your logic to be called on the main
           // application thread
           someControlOnMyForm.Invoke(new             
                      DoUIWorkHandler(_userController.UpdateUsersOnMap);
       }
       else
       {
           _userController.UpdateUsersOnMap()
       }
    }
}
mjmarsh
I think I have done this application quite clumsy. Having UI objects inside my controller objects. So I am not sure if this fits my program. There is also going to be alot of change if I am going to do this I think.
bobjink
+2  A: 

You should remove the while(true), you are adding infinite event handlers and invoking them infinite times.

BioBuckyBall
A: 

You can use the RunWorkerCompleted event on the backgroundWorker class to define what should be done when the background task has completed. So you should do the database call in the DoWork handler, and then update the interface in the RunWorkerCompleted handler, something like this:

BackgroundWorker bgw = new BackgroundWorker();
bgw.DoWork += (o, e) => { longRunningTask(); }

bgw.RunWorkerCompleted += (o, e) => {
    if(e.Error != null && !e.Cancelled)
    {
     _userController.UpdateUsersOnMap();
    }
}

bgw.RunWorkerAsync();
Lee
+1  A: 

in addition to previous comments, take a look at www.albahari.com/threading - best doc on threading you will ever find. It will teach you how to use the BackgroundWorker properly. You should update the GUI when the BackgroundWorker fires Completed event (which is invoked on UI thread to make it easy for you, so that you don't have to do Control.Invoke yourself)

morpheus