0

i have a problem with my code. Im creating a graphic app that have a main form. When i click on a button i create a new form that show a progress bar while export some files. My problem start when i try to close the form with the progress bar because method what export files doesnt end.

In the new form, i execute a method that check the progress of the export and fill the progress bar, this method is executed each second. To export i create a new thread, and this thread execute the export method.

If the export finish, form and thread close right way but if i "force" stop operation by closing the form, the thread that are exporting doesnt stop until the export ends or when i close the main form.

So, how can i stop that thread?

This is my code:

 public Form3(File file, string output, string inputFile)
    {
        InitializeComponent();
        this.file = file;
        this.output = output;
        this.inputFile = inputFile;
        progressLabel.Location = new Point(textProgress.Right, progressLabel.Top);
        thread = new Thread((ThreadStart)delegate { Exporter.ExportToFile(this.file, this.output, this.inputFile); });
        thread.IsBackground = true;
        thread.Start();

        TimerControl();


    }

    private void TimerControl()
    {
        System.Windows.Forms.Timer t = new System.Windows.Forms.Timer();
        t.Tick += new EventHandler(GetProgress);
        t.Interval = 1000; // in miliseconds
        t.Start();    

    }

    private void GetProgress(object sender, EventArgs myEventArgs)
    {
        int x = Exporter.GetProgress();
        progressBar.Maximum = 100;
        if (!Exporter.stop)
        {
            progressBar.Value = x;
            progressLabel.Text = x.ToString() + "%";
        }
        else
        {

            this.Close();

        }
    }
Tibo
  • 49
  • 11
  • To do this properly, you need to add cancellation support to `Exporter.ExportToFile()` – Matthew Watson Nov 04 '16 at 08:53
  • Brutal but (somewhat) effective: In your `Closing` event call `thread.Abort()` - any other methods are of course preferable. – Manfred Radlwimmer Nov 04 '16 at 08:56
  • That is terrible advice. [`Never call Thread.Abort()`](http://stackoverflow.com/questions/1559255/whats-wrong-with-using-thread-abort). – Matthew Watson Nov 04 '16 at 08:57
  • Yep i know that i dont have to use Thread.Abort() thats why i searching other solution. To add cancellation support, i need to send 1 more parameter to my export function? – Tibo Nov 04 '16 at 09:00
  • Yes, you should add a parameter of type `CancellationToken` and use it to signal and respond to cancellation. [See here for more details](https://msdn.microsoft.com/en-us/library/dd997364(v=vs.110).aspx). – Matthew Watson Nov 04 '16 at 09:05

3 Answers3

0

The thing to understand here: that code that runs that "export to file" (Exporter.ExportToFile) ... is probably not written to be stopped.

In other words: you want to enhance that component. If you really mean to use it in such a context, then you definitely want to add another method to your exporter; such as "AbortExport" or something alike!

(instead of just killing the underlying thread for example - you might want to ensure that any file created by that export gets removed for example; and that all associated resources see proper cleanup)

GhostCat
  • 137,827
  • 25
  • 176
  • 248
0

Why not use Tasks instead?

quote from https://msdn.microsoft.com/en-us/library/dd997396(v=vs.110).aspx

By throwing a OperationCanceledException and passing it the token on which cancellation was requested. The preferred way to do this is to use the ThrowIfCancellationRequested method. A task that is canceled in this way transitions to the Canceled state, which the calling code can use to verify that the task responded to its cancellation request.

quote from https://msdn.microsoft.com/en-us/library/dd997364(v=vs.110).aspx

The general pattern for implementing the cooperative cancellation model is:

  • Instantiate a CancellationTokenSource object, which manages and sends cancellation notification to the individual cancellation tokens.

  • Pass the token returned by the CancellationTokenSource.Token property to each task or thread that listens for cancellation.

  • Provide a mechanism for each task or thread to respond to cancellation.

  • Call the CancellationTokenSource.Cancel method to provide notification of cancellation.

See the sample code at the 1st article

George Birbilis
  • 2,782
  • 2
  • 33
  • 35
0

You can use the Thread.Abort(); method , but it is not the optimal/safest way to abort a thread .

The safest way to exit a thread is to add extra code in your exporter class to exit at the right time ..

You can find tons of answers on stackoverflow that agrees with me that threads should be exited safely by user code ..

EDIT : using the Thread.Abort(); is a bad idea , i thought it was clear in the original answer

Yahfoufi
  • 2,220
  • 1
  • 22
  • 41
  • 1
    [No, you should NEVER use `Thread.Abort()`](http://stackoverflow.com/questions/1559255/whats-wrong-with-using-thread-abort). This is extremely bad advice. You should say "you must never use Thread.Abort()", not "You can always use Thread.Abort()". – Matthew Watson Nov 04 '16 at 09:14
  • @MatthewWatson , did you read the whole answer ? or you felt like downvoting the answer ? – Yahfoufi Nov 04 '16 at 09:25
  • Yes I read the whole answer, but the first sentence still says `You can always use the Thread.Abort(); method`. You should make it clear that you should never use that method, not start your answer by saying that you can "always" use it. Can't you see why that's a bad idea? Just remove that entire sentence, or move it to the end and change it to something like "Note that you should never use `Thread.Abort()` to abort a thread". – Matthew Watson Nov 04 '16 at 09:48
  • If the word "always" is the wrong part of the answer , i got it removed .. – Yahfoufi Nov 04 '16 at 09:51