3

Recently we came across some legacy WinForms application we needed to update with some new features. While expert testing the application, it was found that some old functionality is broken. Invalid cross-thread operation. Now before you take me for a newbie, I do have some experience with Windows Forms applications. I'm no expert, but I consider myself experienced with threading and know exactly how to manipulate GUI components from a worker thread.

Apparently the one who wrote this piece of software did not and just set UI control properties from a background thread. Of course it threw the usual exception, but it was encased in an all-catching try-catch block.

I spoke with a stakeholder to find out how long this functionality had been broken, and turned out it was fine before. I couldn't believe that, but he demoed me that the function actually works in PROD.

So our unrelated updates "broke" the software in three places, but I can't get my head around how it could have worked in the first place. According to source control, the bug has always been there.

Of course we updated the application with proper cross-thread UI handling logic, but now I'm in the unlucky position of having to explain to stakeholders with not much tech background why something that can't have worked like ever, did work fine until we did some unrelated changes to the system.

Any insights would be appreciated.

One more thing that came to mind: in two of the cases the background thread updated a ListView control that was on a non-selected (and thus non-displayed) TabPage control. The third time it was standard Label control (text property was set) that was initially empty and gets a Text assigned according to what the background thread finds.

Here's some code for you, very similar to the one we found:

private void form_Load(object sender, System.EventArgs e)
{
  // unrelated stuff...
  ThreadStart ts = new ThreadStart(this.doWork);
  Thread oThread = new Thread(ts);
  ts.Start();
  // more unrelated stuff ...
}

public void doWork()
{
  string error = string.Empty;
  int result = 0;
  try
  {
    result = this.service.WhatsTheStatus(out error); // lengthy operation
    switch (result)
    {
      case 1:
        this.lblStatus.Text = "OK";
        break;
      case -1:
        this.lblStatus.Text = "Error";
        this.lblError.Text = error;
        break;
      default:
        this.lblStatus.Text = "Unknown";
        break;
    }
  }
  catch
  {
  }
}

Unfortunately I saw lblStatus being updated in the production environment and it isn't referenced from anywhere else in the entire application (except the Designer generated stuff, of course).

gaborzo
  • 167
  • 10
  • Why is there ``oThread`` if it isn't started? – Binkan Salaryman Sep 16 '15 at 08:20
  • Does your real code uses `Thread` or `Task`? Tasks are not quarantied to run in a separate thread [see here](http://stackoverflow.com/questions/12245935/is-task-factory-startnew-guaranteed-to-use-another-thread-than-the-calling-thr). Maybe this is the reason why it runs in older code. – Sebastian Schumann Sep 16 '15 at 08:23
  • 1
    "something that can't have worked like ever" is a mis-characterization. It's possible to sharpen a pencil using a chainsaw. It doesn't mean that it's safe, or recommended, or will *always* work. That's what the newer checks are there to protect you from - that accessing UI elements from other threads was never *safe*, but it may have *happened* to work on previous occasions. – Damien_The_Unbeliever Sep 16 '15 at 08:42
  • @Binkan When providing the code I was renaming functions and variables and looks like I was sloppy. Of course it's the thread that is being started. – gaborzo Sep 16 '15 at 08:53

1 Answers1

3

That is because cross-thread access exceptions are not guaranteed to be thrown when you run code without debugger (in windows forms). Read this for example: https://msdn.microsoft.com/en-us/library/vstudio/ms171728%28v=vs.110%29.aspx:

The .NET Framework helps you detect when you are accessing your controls in a manner that is not thread safe. When you are running your application in the debugger, and a thread other than the one which created a control tries to call that control, the debugger raises an InvalidOperationException with the message, "Control control name accessed from a thread other than the thread it was created on."

This exception occurs reliably during debugging and, under some circumstances, at run time. You might see this exception when you debug applications that you wrote with the .NET Framework prior to the .NET Framework 2.0. You are strongly advised to fix this problem when you see it, but you can disable it by setting the CheckForIllegalCrossThreadCalls property to false. This causes your control to run like it would run under Visual Studio .NET 2003 and the .NET Framework 1.1.

You can check this yourself by writing simple winforms application, set form title from another thread and see exception is thrown. Then run same application without debugger attach and see how form title will happily change without any problems.

Community
  • 1
  • 1
Evk
  • 98,527
  • 8
  • 141
  • 191
  • "without any problems" - except that there could be - on rare occasions. That's the real point - it's unsafe, but not guaranteed to always blow up in your face. – Damien_The_Unbeliever Sep 16 '15 at 08:50
  • @Damien_The_Unbeliever sure - I just mean it will not throw exception in most cases (which is very bad, WPF does not have this problem for example). – Evk Sep 16 '15 at 08:53