C# Error creating window handle on Environment.Exit call

Question

I've created a simple thread controller class managing the thread's execution, here its code:

 public class ThreadController {
  int waitCount;
  Thread myThread;

  public ThreadController() {
    //
  }

  public void StartThread() {
    waitCount = 0;
    // launch a thread to show an alert when conditions are met!
    myThread = new Thread(new ThreadStart(ThreadAction));
    myThread.IsBackground = true;
    myThread.Start();
  }

  // method is async as it call an async method itself!
  void ThreadAction() {
    while (myThread.IsAlive) {
      Thread.Sleep(5000);
      bool doStop = DoStopTest().Result; // some async function testing stop criterion
      if (doStop) {            
        MainForm.BeginInvoke(new MethodInvoker(delegate() {
            MessageBox.Show("Thread stopped!");
          }));
        // 
        myThread.Abort();
      }
      ++waitCount;
      if (waitCount >= 15) {
        myThread.Abort();
      }
      Thread.Sleep(5000);
    }
  }
}

Now, I want to make sure the above created threads (there might be several) are killed when I close the MainForm, which I read should be done in the FormClosing event as follows:

void Main_FormClosing(object Sender, FormClosingEventArgs e) {
  // unfortunately, an error is thrown when I call following line...
  Environment.Exit(Environment.ExitCode);
}

The Environment.Exit call actually generates some weird exceptions... Sometimes a "vhost32.exe stopped working", sometimes an error System.ComponentModel.Win32Exception (0x80004005): Error creating window handle or other painting events that use "Invalid Parameters"...

Am I missing something here? What is the suggested way to cleanly close the form with all associated threads, without running into errors?

Answer 1

The code would be a lot clearer if you used tasks and async/await. DoStopTest() seems to return a Task already, so there's no need to use a raw Thread.

The code could be something as simple as a loop :

public async Task MyTestAndWait()
{
    await Task.Delay(5000);
    var waitCount=0;
    while( waitCount++ < 15 && !(await DoStopTest()))
    {
        await Task.Delay(10000);

    }
    MessageBox.Show("Thread stopped!");
}

After each call to await execution resumes on the original synchronization context. For desktop applications, that's the UI thread. That means there's no need to use BeginInvoke

Threads should not be aborted. The correct way is to check a thread-safe signal, like a ManualResetEvent that's raised when a thread needs to exit. When signalled, the thread's code itself should exit.

Using a lot of events can get a bit messy which is why .NET 4.5 added the CancellationToken and CancellationTokenSource classes that can be used to notify both threads and Tasks they need to cancel and exit gracefully.

public async Task MyTestAndWait(CancellationToken ct,int initialDelay,int pollDelay)
{
    await Task.Delay(initialDelay,ct);
    var waitCount=0;
    while(!ct.IsCancellationRequested && waitCount++ < 15 && !(await DoStopTest()))
    {
        await Task.Delay(pollDelay,ct);

    }
    MessageBox.Show("Poll stopped!");
}

This will cancel the delays and the loop but it won't cancel the call to DoStepTest(). That method will have to accept a CancellationToken parameter as well

CancellationTokens are created by CancellationTokenSource classes. One of the overloads accepts a timeout, which could be used to cancel the overall operation :

public async void SendSMS_Click(object sender, EventArgs args)
{
    var cts=new CancellationTokenSource(TimeSpan.FromMinutes(15));
    await MyTestAndAwait(cts.Token,5000,10000);       
}

The cts could be stored in a field, to allow cancellation due to another event like a button click :

CancellationTokenSource _cts;
public async void SendSMS_Click(object sender, EventArgs args)
{
    SendSMS.Enabled=false;
    Cancel.Enabled=true;

    _cts=new CancellationTokenSource(TimeSpan.FromMinutes(15);
    await MyTestAndAwait(cts.Token,5000,10000);       
    _cts=null;

    SendSMS.Enabled=true;
    Cancel.Enabled=false;    
}

public async void Cancel_Click(object sender, EventArgs args)
{
    _cts?.Cancel();
}

The same code can be used to signal cancellation when closing the form :

void Main_FormClosing(object Sender, FormClosingEventArgs e) 
{
    _cts.?Cancel();
}

BTW there's no reason to call Environment.Exit() in the form's Closing or Closed events. Closing the main form will end the application unless there's another thread running.

UPDATE

It looks like the actual question is how to verify that an SMS was sent by polling for its send status. The code in this case would be different, while still using task. The method shouldn't have any reference to the UI so it can be moved to a separate Service-layer class. After all, changing providers shouldn't result in changing UIs

Assuming HttpClient is used, it could look like this :

//In an SmsService class

public async Task<(bool ok,string msg)> SendSmsAsync(string phone,string message,CancellationToken ct)
{
    var smsMsg=BuildSmsContent(phone,string);
    await _httpClient.PostAsync(smsMsg,ct);

    //wait before polling
    await Task.Delay(_initialDelay,ct);
    for(int i=0;i<15 && !ct.IsCancellationRequested;i++)
    {
        var checkMsg=CheckStatusContent(phone,string);
        var response=await _httpClient.GetAsync(check,ct);

        if (ct.IsCancellationRequested) break;

        //Somehow check the response. Assume it has a flag and a Reason
        var status=ParseTheResponse(response);
        switch(status.Status)
        {
            case Status.OK:
                return (ok:true,"Sent");
            case Status.Error:
                return (ok:failed,status.Reason);
            case Status.Pending:
                await Task.Delay(_pollDelay,ct);
                break;
        }
    }
    return (ok:false,"Exceeded retries or cancelled");    
}

This method could be used from a button event :

CancellationTokenSource _cts;

public async void SendSMS_Click(object sender, EventArgs args)
{
    DisableSending();

    var phone=txtPhone.Text;
    var message=txtMessage.Text;
    _cts=new CancellationTokenSource(TimeSpan.FromMinutes(15);

    var (ok,reason)=await _smsService.SendSmsAsync(phone,message,cts.Token);       
    _cts=null;

    if (ok)
    {
        MessageBox.Show("OK");
    }
    else
    {
        MessageBox.Show($"Failed: {reason}");
    }

    EnableSending();
}

public void EnableSending()
{
    SendSMS.Enabled=true;
    Cancel.Enabled=false;    
}

public void DisableSending()
{
    SendSMS.Enabled=false;
    Cancel.Enabled=true;    
}