views:

797

answers:

5
  1. In a simple winform application, I call a function that endlessy create files on a button click event. I add Application.DoEvents() to the loop.

  2. I press the red X to close the form.

  3. the form closes, but files continue to be created ...

I think its on the buttons thread, but shouldnt it be a background one ? trying changing Thread.CurrentThread.IsBackGround to True on the loop function does not help.

Ideas ?

+4  A: 

The fact that you're using Application.DoEvents is the first sign of a problem: it shows that you're doing too much in the UI thread. It's almost never appropriate in a well-structured program. The UI thread is not meant to have any long-running tasks. (Admittedly if it takes a long time to draw your UI you have little choice - but that suggests you should simplify your UI... and it's not applicable in this case, I suspect.)

Instead, you should be performing the long-running task (creating the files) in a separate thread. BackgroundWorker is a perfect fit for this - you can use it to report progress back to the UI, and the UI can call CancelAsync method to request that it stops. You need to check the CancellationPending property from within the worker thread, to see whether cancellation has been requested, and stop appropriately.

EDIT: Just to clarify what I believe is happening - I suspect your form is closing, but the program won't terminate until the event loop has finished. You're keeping the event loop going with your file-creation loop, hence the problem.

Note that there isn't a thread for the button - there's just one for your whole UI. (In certain cases you may need more than one UI thread, but that's rare - and you'd know it if you'd done it.)

Jon Skeet
thank you for your time. I know there are different and better ways to do it, I want to understand why this one is not working (optimal or not)
gil
Did my edit help? Basically until you let the button click handler finish, the application isn't going to stop (unless you use something horrible like Thread.Abort)
Jon Skeet
OK, just one more Q, you said "Note that there isn't a thread for the button - there's just one for your whole UI" :: when i press the red X the debugger jumps to the disposing. on what thread is this done ? as the UI's one is busy looping ...
gil
It's in the UI thread - look at the stack and you'll see your file creation loop is in there as well - the call to Application.DoEvents will end up handling the X click and thus the form closing. It's all in the one thread - which is nasty, hence the recommendation to avoid it :)
Jon Skeet
Jon, are you actually living on SO? Your answers read as if you were writing them for a book, all within 2-3 minutes from the time soneone asks a question. Incredible.
Alan
(...and this is meant to be positive, no bashing or cynicism at all :)
Alan
@Alan: I guess I do hit refresh a bit too often... but I'm getting other things done too. Time for a bit of Mario Kart now, for instance...
Jon Skeet
A: 

Did you specifially create a new thread for that ButtonClick code? If not, then it's on the same thread as the Form, so the "X" button should stop it.

In either case, you could set a flag (isClosing), and check for that flag in your "while true" loop.

Timothy Khouri
so the "X" button should Form stop it :: OK, but it doesnt. try it ...
gil
No, it shouldn't stop it - because you've still got the loop running.
Jon Skeet
@Timothy: How would you expect the X button to stop it? At some point in the stack for the UI thread we've got this never-ending loop. How is that stack going to be unwound? It would have to either be due to that handler returning, or through an exception. Why would an exception be thrown?
Jon Skeet
A: 

Hi,

Did you try to call Thread.CurrentThread.Abort()? It is not the recommended way, but if you don't have access to the functions source code it might help (even though you say you are calling DoEvents` from the function).

If you do have access to the source code, then you could put a check in there, to let the loop know that it has to stop.

Also, you could use the background worker component, or create a different thread, and control it from outside (i.e. from the form)

Bogdan Maxim
Calling Thread.Abort is almost always the wrong solution - as is calling Application.DoEvents. Better to get rid of the latter than to add the former...
Jon Skeet
+1  A: 

A button click handler executes on the UI thread.

When you call Application.DoEvents, you let the UI thread process window messages. One effect of this is that each time your user clicks that button, a new 'instance' (stack frame) of the click handler method, along with a lot of other stack frames, get added to the stack. Like this:

{windows forms methods incl. message pump}
  **ClickHandler**
    Application.DoEvents
     {windows forms methods incl. message pump}
      **ClickHandler**
       Application.DoEvents
        {windows forms methods incl. message pump}
          etc.

It's all pretty horrible.

If you want to run code in the background, better alternatives are to use

  • The BackgroundWorker component.
  • The standard .NET delegate-based mechanism (BeginInvoke, EndInvoke, IAsyncResult, etc) for running code in the thread pool.
  • If you want to implement a long-running background thread, rather than hive off a 'chunk of work' to the pool, you could create and Start a Thread, which is set to be a background thread.
mackenir
+1  A: 

Add this form-level variable to your form's code:

private bool _StillOpen = true;

Then wrap the endless-loop code in your button click like this:

while (_StillOpen)
{
    // do whatever your method does
    Application.DoEvents();
}

Finally, add this code to your form's FormClosing event:

_StillOpen = false;

This will allow your form to close when you click the close button. Ideally you would want something like this to execute on a background thread, but this flag approach may be a quick fix to your current problem.

MusiGenesis