2

I have a situation that i export data to a file and what i have been asked to do is to provide a cancel button which on click will stop the export if it takes too much time to export.

I started exporting to the file in a thread. And i try to abort the thread on the button click. But it do not work.

I searched on Google and i found that abort() is not recommended. But what else should I choose to achieve it?

My current code is:

private void ExportButtonClick(object param)
{
    IList<Ur1R2_Time_Points> data = ct.T_UR.ToList();
    DataTable dtData = ExportHelper.ToDataTable(data);
    thread = new Thread(new ThreadStart(()=>ExportHelper.DataTableToCsv(dtData, "ExportFile.csv")));
    thread.SetApartmentState(ApartmentState.STA);
    thread.IsBackground = true;
    thread.Name = "PDF";
    thread.Start();
}

private void StopButtonClick(object param)
{
    if (thread.Name == "PDF")
    {
        thread.Interrupt();
        thread.Abort();
    }
}
Jason Evans
  • 28,906
  • 14
  • 90
  • 154
  • 3
    You need to fix the bug that causes it to take too long to save the file, not call `thread.Abort()` which is likely to end in tears. – Matthew Watson Feb 16 '17 at 08:47
  • Use cancellation over Abort. http://stackoverflow.com/questions/3632149/question-about-terminating-a-thread-cleanly-in-net – Eldho Feb 16 '17 at 08:50
  • @MatthewWatson Its a big file.There is no bug – tosttest tosttest Feb 16 '17 at 08:53
  • what is `ExportHelper` and what does `DataTableToCsv` do? – default Feb 16 '17 at 08:55
  • @tosttesttosttest - Calling `Thread.Abort()` can leave the run-time in an unknown state and as such that **only time** you should ever do it is when you are trying to abort your entire application. The only safe way to abort is to run your code in a **separate process** and then kill that process if needed. – Enigmativity Feb 16 '17 at 08:58
  • ExportHelper is a class and DataTableToCsv is a function. I pass Datatable and i create .csv file corresponding to that datatble. – tosttest tosttest Feb 16 '17 at 08:59
  • I understand that. I'm asking about the implementation – default Feb 16 '17 at 08:59
  • @Enigmativity your answer about process sounds interesting Could please explain me in detail below so taht i mark you as answer (With how i should adapt my code to achieve it as i see generally process is used to launch the .exe files) – tosttest tosttest Feb 16 '17 at 09:01
  • @tosttesttosttest *why* do you want to abort the thread instead of exiting it if it takes too long? Besides, why would a file save take too long? Shouldn't you try to fix the real bug? – Panagiotis Kanavos Feb 16 '17 at 09:04
  • @PanagiotisKanavos as i am generating csv file from datatable and datatable could be having 800 000 000 000 rows. On this case if it takes times then user wil have option to stop it and filter the datatable to reduce teh size of datatable. – tosttest tosttest Feb 16 '17 at 09:07
  • 3
    What does `DataTableToCsv` do? Please post its source code. *That's* the method that should be modified to allow for cancellation, not the enclosing thread. BTW why not use `Task.Run` instead of a raw thread? – Panagiotis Kanavos Feb 16 '17 at 09:07
  • 3
    @tosttesttosttest then you are definitely doing it wrong - generating 800 *billion* rows from a single datatable? Even if the number is correct, you should be using the database's export facilities. All of them have such facilities. – Panagiotis Kanavos Feb 16 '17 at 09:09
  • It doesn't matter whether you want to export 80K or 800Bn rows. You are still looping over tables and rows. `DataTableToCsv` should be modified to allow for cancellation, ideally by checking a [CancellationToken](https://msdn.microsoft.com/en-us/library/system.threading.cancellationtoken(v=vs.110).aspx). – Panagiotis Kanavos Feb 16 '17 at 09:10
  • Finally, *WHY* are you trying to export 800Bn rows to a CSV? Wouldn't it be easier to just copy the database? Or set up replication? Or backup the database and restore it on another machine? – Panagiotis Kanavos Feb 16 '17 at 09:11
  • Cancellation is partially why Task is better than raw thread. Learn how Task works... – dragonfly02 Feb 16 '17 at 09:16
  • Yes, I understand and already know what you have all said. But i have to do it because i am purposed to do it and left with no more options. Could you please let me know more about Cancellation token – tosttest tosttest Feb 16 '17 at 09:18
  • Post the source to `DataTableToCsv`. *That's* the method that needs cancellation, not the thread. – Panagiotis Kanavos Feb 16 '17 at 09:19
  • BTW, why don't you *check* the size of the datatable first and ask the user for confirmation before proceeding? And why call `ToList`? It looks like you are using MoreLINQ's ToDataTable extension method. That method accepts IEnumerable. You are wasting memory by converting the data to a list first, DataTable second. In fact, you should be able to write out the `IEnumerable` without creating either the list or the datatable. – Panagiotis Kanavos Feb 16 '17 at 09:32
  • @PanagiotisKanavos I humbly request to answer the question asked. I would really appreciate if you could please let me tell a solution of the problem that when i click the "Cancel" button then it must stop the .csv creation from datatable by any way. – tosttest tosttest Feb 16 '17 at 10:02
  • @tosttesttosttest I already did in the comments. Posted as answer too. But you *really* need to fix the inefficiencies. These are bad enough to cause 10x-100x delays. For example, are you *sure* the delay isn't caused by multiple object allocations when you copy data from the IEnumerable to the list and *then* to the DataTable? List buffers are grown by allocating another buffer with twice the capacity and copying the original data. That results in **A LOT** of wasted allocations. Converting that to a DataTable simply doubles the CPU and RAM waste – Panagiotis Kanavos Feb 16 '17 at 10:08
  • `Its a big file.There is no bug ` Well there will be once you start calling `Thread.Abort()`! – Matthew Watson Feb 16 '17 at 10:56
  • I solved it using Token cancellation in the DataTableToCsv() returning cancel.ThrowIfCancellationRequested(); – tosttest tosttest Feb 16 '17 at 11:10

4 Answers4

3

Aborting a thread is a bad idea, especially when dealing with files. You won't have a chance to clean up half-written files or clean-up inconsistent state.

It won't harm the .NET Runtime bat it can hurt your own application eg if the worker method leaves global state, files or database records in an inconsistent state.

It's always preferable to use cooperative cancellation - the thread periodically checks a coordination construct like a ManualResetEvent or CancellationToken. You can't use a simple variable like a Boolean flag, as this can lead to race conditions, eg if two or more threads try to set it at the same time.

You can read about cancellation in .NET in the Cancellation in Managed Threads section of MSDN.

The CancellationToken/CancellationTokenSource classes were added in .NET 4 to make cancellation easier that passing around events.

In your case, you should modify your DataTableToCsv to accept a CancellationToken. That token is generated by a CancellationTokenSource class.

When you call CancellationTokenSource.Cancel the token's IsCancellationRequested property becomes true. Your DataTableToCsv method should check this flag periodically. If it's set, it should exit any loops, delete any inconsistent files etc.

Timeouts are directly supported with CancelAfter. Essentially, CancelAfter starts a timer that will fire Cancel when it expires.

Your code could look like this:

CancellationTokenSource _exportCts = null;

private void ExportButtonClick(object param)
{
    IList<Ur1R2_Time_Points> data = ct.T_UR.ToList();
    DataTable dtData = ExportHelper.ToDataTable(data);

    _exportCts=new CancellationTokenSource();
    var token=_exportCts.Token;

    thread = new Thread(new ThreadStart(()=>
            ExportHelper.DataTableToCsv(dtData, "ExportFile.csv",token)));
    thread.SetApartmentState(ApartmentState.STA);
    thread.IsBackground = true;
    thread.Name = "PDF";

    _exportCts.CancelAfter(10000);
    thread.Start();

}


private void StopButtonClick(object param)
{
    if (_exportCts!=null)
    {
        _exportCts.Cancel();
    }
}

DataTableToCsv should contain code similar to this:

foreach(var row in myTable)
{
    if (token.IsCancellationRequested)
    {
        break;
    }
    //else continue with processing
    var line=String.Join(",", row.ItemArray);
    writer.WriteLine(line);

}

You can clean up your code quite a bit by using tasks instead of raw threads:

private async void ExportButtonClick(object param)
{
    IList<Ur1R2_Time_Points> data = ct.T_UR.ToList();
    DataTable dtData = ExportHelper.ToDataTable(data);

    _exportCts=new CancellationTokenSource();
    var token=_exportCts.Token;

    _exportCts.CancelAfter(10000);
    await Task.Run(()=> ExportHelper.DataTableToCsv(dtData, "ExportFile.csv",token)));
    MessageBox.Show("Finished");
}

You could also speed it up by using asynchronous operations, eg to read data from the database or write to text files without blocking or using threads. Windows IO (both file and network) is asynchronous at the driver level. Methods like File.WriteLineAsync don't use threads to write to a file.

Your Export button handler could become :

private void ExportButtonClick(object param)
{
    IList<Ur1R2_Time_Points> data = ct.T_UR.ToList();
    DataTable dtData = ExportHelper.ToDataTable(data);

    _exportCts=new CancellationTokenSource();
    var token=_exportCts.Token;

    _exportCts.CancelAfter(10000);
    await Task.Run(async ()=> ExportHelper.DataTableToCsv(dtData, "ExportFile.csv",token)));
    MessageBox.Show("Finished");
}

and DataTableToCsv :

public async Task DataTableToCsv(DataTable table, string file,CancellationToken token)
{
...
    foreach(var row in myTable)
    {
        if (token.IsCancellationRequested)
        {
            break;
        }
        //else continue with processing
        var line=String.Join(",", row.ItemArray);
        await writer.WriteLineAsync(line);
    }
Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
  • tip: either disable the `ExportButtonClick` if an export is currently running or do `_exportCts?.Cancel()` before the `_exportCts=new CancellationTokenSource()` so that it is cancelled (if it exists) before recreating it. – default Feb 16 '17 at 10:30
  • also be sure to send the token to the `Task.Run(() => {}, token)` so that it is associated with the task. I can't say why but I have read threads here on SO that it is recommended. – default Feb 16 '17 at 10:31
  • @Default 1) that's used to prevent the task from starting if the CTS is already cancelled, set the Task's status to `Canceled` instead of `Faulted` or `RanToCompletion` and return a `TaskCanceledException` if [CancellationToken.ThrowIfCancellationRequested()](https://msdn.microsoft.com/en-us/library/system.threading.cancellationtoken.throwifcancellationrequested(v=vs.110).aspx). That's explained in [Task Cancellation](https://msdn.microsoft.com/en-us/library/dd997396(v=vs.110).aspx). – Panagiotis Kanavos Feb 16 '17 at 11:04
  • @Default 2) adding that here would result in a very long answer and a contrived example. if we had the source to `DataTableToCsv()` we could rewrite it *and* the event handler to clean up, handle cancellation exceptions in a different way than faults etc. – Panagiotis Kanavos Feb 16 '17 at 11:14
  • Thanks i solved it the same way. Tank you so much @PanagiotisKanavos – tosttest tosttest Feb 16 '17 at 11:14
  • @tosttesttosttest then make sure you pass the token to Task.Run as `Default` suggested. – Panagiotis Kanavos Feb 16 '17 at 11:14
  • @PanagiotisKanavos sorry, I'm not quite sure how to respond to your comments.. But if I interpret you correctly, my first comment was referring to; if the method for example writes to the same file, the `_exportCts?.Cancel()` would simply trigger cancel on the previous task started. Since you do `_exportCts = new..` the new CancellationTokenSource is **not** cancelled. 2) Not expecting you to explain it, just to add it to your example would be nice. – default Feb 16 '17 at 13:40
2

You can use a boolean flag. Use a volatile boolean for that.

In the helper do something like:

 this.aborted = false;
 while(!finished && !aborted) {
      //process one row
 }

Whenever you want to cancel the operation, you call a method to set aborted to true:

 public void Abort() {
     this.aborted = true;
 }
Fernando
  • 396
  • 1
  • 5
  • Yeah i have already seen this. But in my case i do not have infinite loop like while(true) here and u set false when you have to abort .But i am exporting a file so no while loop to put condition to false to break infinite iteration. So dont know how to set this file condition in my case because i am doing " ThreadStart(()=>ExportHelper.DataTableToCsv(dtData, "ExportFile.csv")));" – tosttest tosttest Feb 16 '17 at 08:49
  • @tosttesttosttest then check as often as needed. Don't use a Boolean BTW, that's asking for a race condition. .NET has the CancellationTokenSource and CancellationToken classes for this. – Panagiotis Kanavos Feb 16 '17 at 09:03
0

Have a read here: https://msdn.microsoft.com/en-us/library/system.threading.threadabortexception(v=vs.110).aspx

When a call is made to the Abort method to destroy a thread, the common language runtime throws a ThreadAbortException. ThreadAbortException is a special exception that can be caught, but it will automatically be raised again at the end of the catch block. When this exception is raised, the runtime executes all the finally blocks before ending the thread. Because the thread can do an unbounded computation in the finally blocks or call Thread.ResetAbort to cancel the abort, there is no guarantee that the thread will ever end. If you want to wait until the aborted thread has ended, you can call the Thread.Join method. Join is a blocking call that does not return until the thread actually stops executing.

Since Thread.Abort() is executed by another thread, it can happen anytime and when it happens ThreadAbortException is thrown on target thread.

Inside ExportHelper.DataTableToCsv:

catch(ThreadAbortException e) {
    Thread.ResetAbort();
}

On StopButtonClick

if (thread.Name == "PDF")
{
    thread.Interrupt();
    thread.Join();
}
Sandip Bantawa
  • 2,822
  • 4
  • 31
  • 47
0

To Stop a thread you have one option of Thread.Abort.However because this method thrown ThreadAbortException on the target thread when it executed by another thead. Which is not recommended. The second option to stop a thread is by using shared variable that both your target and your calling thread can access. See the Example ::

public static class Program
{
    public static void ThreadMethod(object o)
    {
        for (int i = 0; i < (int)o; i++)
        {
            Console.WriteLine("ThreadProc: { 0}", i);
            Thread.Sleep(0);
        }
    }
    public static void Main()
    {
        bool stopped = false;
        Thread t = new Thread(new ThreadStart(() =>
        {
            while (!stopped)
            {
                Console.WriteLine("Running...");
                Thread.Sleep(1000);
            }
        }));
        t.Start();
        Console.WriteLine("Press any key to exit");
        Console.ReadKey();
        stopped = true;
        t.Join();
    }
}

//Source :: Book --> Programming in c#

Enigmativity
  • 113,464
  • 11
  • 89
  • 172
M.Abdullah
  • 61
  • 2
  • Please don't ever say that `Thread.Abort()` is an option. It's not. Ever. – Enigmativity Feb 16 '17 at 09:22
  • I suspect that book says that `Thread.Abort` is a very bad idea and cooperative cancellation is the only real alternative. If it's an old book, it will talk about ManualResetEvent. Newer books will talk about CancellationToken – Panagiotis Kanavos Feb 16 '17 at 09:29