views:

388

answers:

5

Hello.

I´m developing a software in C# that uses static functions from a C++ .dll file through a Wrapper.

The problem is that some of these functions are slow and unstable, so in order to solve this I created a Thread that executes them. However, when I abort that thread from the main thread, the program isn´t letting me use those functions again, even though I define a new instance of the thread every time I call a function.

Is there any way I can fix this?

Thanks in advance.

PS: Here is some code from my program:

public partial class MainForm : Form, IMultiLanguage
{
      //...

      //This method is subscribed to the event of pressing the 'Abort' button
      private void StopCurrentAnalisis()
      {
            try
            {
                this.analisisManagerController.AnalisisThread.Abort();
            }
            catch (Exception e){  }
            finally
            {
                MessageBox.Show("Analisis has been cancelled by user", "Analisis Interrupted", MessageBoxButtons.OK, MessageBoxIcon.Stop);
                CerrarNowLoadingForm();
            }
      }

      //..
}

public class AnalisisManager: IAnalisisManagerController
{
    //..
    private Thread analisisThread;
    public Thread AnalisisThread{get{return this.analisisThread;}}


    public void MakePadrobAnalisis(TipoAnalisis tipoAnalisis,
        Dictionary<string, Dictionary<int, double>> parametros)
    {
        object[] arregloParams = new object[]{tipoAnalisis,parametros};
        analisisThread = new Thread(new ParameterizedThreadStart(MakeAnalisisInOtherThread));
        analisisThread.Start(arregloParams);
    }

 private void MakeAnalisisInOtherThread(object o)
    {
        object[] arregloParams = o as object[];
        TipoAnalisis tipoAnalisis = (TipoAnalisis) arregloParams[0];
        Dictionary<string, Dictionary<int, double>> parametros = arregloParams[1] as Dictionary<string, Dictionary<int, double>>;

        //This launches an event telling the GUI the unstable analisis has started.
        //The method that makes the 'Abort' button to appear on the GUI is subscribed to this event
        StartAnalisis();

       //The Thread executes DLL functions according to tipoAnalisis, for example:
       case InvKinematicsMinTorque:
       {
           WrapperPadirob.InverseKinematicsMinTorqueConfigAnalisis();
           break;
       }
       //..
   }
}
+5  A: 

However, when I abort that thread from the main thread

That's your problem. You should not be using Thread.Abort. Check out "Non-Evil Cancellation" in this article for a better approach. From there ...

The approach I always recommend is dead simple. Have a volatile bool field that is visible both to your worker thread and your UI thread. If the user clicks cancel, set this flag. Meanwhile, on your worker thread, test the flag from time to time. If you see it get set, stop what you're doing.

Also, very good discussion of this in this SO thread.

Per comment:

You own this thread ...

Thread AnalisisThread{get{return this.analisisThread;}}

... it executes a delegate pointing to ...

private void MakeAnalisisInOtherThread(object o)

... you also own that method. If the C++ interface does not have a reasonable .StopAnalysis method (or something similar), you must wait until the analysis is done and then poll the main thread as discussed in the articles. Blowing up your thread to stop the analysis is not the right thing to do.

JP Alioto
I´m looking forward making this kind of solution, but how can i make the Thread read the bool value if it is executing a C++ library to which i have no access?. Thanks
@JP, any comments on my answer?
Sam Saffron
+1  A: 

BTW, is your C++ dll written to work in multiple threads? It's possible that it uses techniques that only work when it's being used with only one thread. Run it in multiple threads, and very nasty things could happen, like different users accessing each others data, or the code overwriting the C runtime library heap.

John Saunders
+1  A: 

Rather than creating and killing threads all the time, it's better to create one thread which runs code like this:

private void Worker()
{
    while (m_bRunning)
    {

         m_autoResetEvent.WaitOne();
         DoWork(); // eg. call all required functions, maybe by popping requests off a queue
    }
}

Then, you can Set the auto-reset event which will wake the thread from suspend and have it service one request.

This has the additional benefit that all calls will be made to the DLL from a single thread.

Nick
A: 

A slick way to allow a method call to timeout for whatever reason is to use a delegate call.

[DllImport ("mydll.dll")]
private extern void method_call ();

private void thing_1 ()
{
  dll_method_call failing_method;
  IAsycResult method_result;
  Int32 method_timeout;

  method_timeout = 15000;
  failing_method = method_call;
  method_result = failing_method.BeginInvoke (null, null);
  if (method_result.AsyncWaitHandle.WaitOne (method_timeout))
  {
    method_result.EndInvoke ();
    //Success
  }
  else
  {
    //Failure
  }
}

private delegate void dll_method_call ();
+1  A: 

To recap: you have some unmanaged code that is unstable, which you need to consume from your C# app and are looking for the best way to consume this.

Sorry to say this, but your current design is flawed, if you abort a thread while its running some arbitrary unmanaged code you are risking corrupting your application. Handles can leak, memory can leak, you can cause all sorts of incredibly hard to debug concurrency issues.

To solve this kind of problem I would recommend:

  • Best solution: get the unmanaged dll fixed.
  • Second best solution: run a separate process whose sole responsability is to interact with the problem dll, if stuff plays up, terminate the process and then start it up again. Have your main app communicate with that process using remoting or WCF
Sam Saffron
Sam, I agree. The extra level of indirection might be enough to allow the process to finish without having to blow it up. Bottom line is that you are very correct that blowing up the thread unceremoniously is a recipe for unmanaged disaster. :) +1
JP Alioto