Multithreading done "right"

Fedor Hajdu

I recently started working for a new company and found quite a few situations like this:

Thread m_thread = new Thread(new ThreadStart(GenerateTimeLineImage));
m_thread.Start();
m_thread.Join();

I'm no expert on mulithreading but there are so many different (not just brainless copy-paste) of code like that, that it made me wondering is there some hidden magic behind that code that I'm not aware of. So, what's the difference between that and

GenerateTimeLineImage();

besides obvious performance loss due to creating a pointless thread?

User 8592736

I dont think you missed the magic here. I think, you are right that the thread is pointless in this sample. You should speak to the author of this code if hes still in your company.

Fedor Hajdu

He's not, I'm his replacement... :)

TorstenH

That's why you are replacing him - you can see it.

regards Torsten When I'm not working

Manfred Rudolf Bihy

Yes you are right that this code works synchronously albeit using a separate thread. The coder needs to stand in the corner with a dunce cap on. Cheers! :laugh:

"With sufficient thrust, pigs fly just fine."

Ross Callon, The Twelve Networking Truths, RFC1925

PIEBALDconsult

I think that makes sense in a WinForms app -- if GenerateTimeLineImage takes a bit of time, which seems likely.

Lost User

The good man heard that he was supposed to start a thread when some task takes longer. He did that. Nobody told him what to do once he got the thread going. :) Good soldiers who obey every order rarely make good thinkers :)

At least artificial intelligence already is superior to natural stupidity

Fedor Hajdu

It's not a winforms app, but even it were, it still don't make sense. GenerateTimeLineImage is a heavy function but it would still block the UI thread (if that's why you mentioned WinApp) because of the m_thread.Join() there.

Lost User

No, not the least. In that case the UI thread would be waiting until the other thread is finally done. Meanwhile the UI would not respond, just as if the task had been done on the UI thread itself.

At least artificial intelligence already is superior to natural stupidity

PIEBALDconsult

Thread.Join Method Blocks the calling thread until a thread terminates, while continuing to perform standard COM and SendMessage pumping.

BobJanova

Does this actually work? (I.e. do your event handlers get called, does the window repaint properly, etc?)

User 8592736

No. Try the following code - The ui blocks for 10 seconds - i am sure.

private void button1_Click(object sender, EventArgs e)
{
Thread m_thread = new Thread(new ThreadStart(GenerateTimeLineImage));
m_thread.Start();
m_thread.Join();
}

void GenerateTimeLineImage()
{
Thread.Sleep(10000);
}

PIEBALDconsult

OK, I sit corrected. However, maybe the developer often needs to do that with several items:

Thread t1 = ... ; t1.Start() ;
Thread t2 = ... ; t2.Start() ;
...
Thread tn = ... ; tn.Start() ;

Join (t1 ) ;
Join (t2 ) ;
...
Join (tn ) ;

and uses this as a general pattern. :shrug: In which case I might consider a method that takes a collection of delegates:

private static void
InvokeAll
(
  params System.Threading.ThreadStart\[\] Items
)
{
  /\* Null checking  :D  \*/

  System.Threading.Thread\[\] thread = new System.Threading.Thread \[ Items.Length \] ;

  for ( int i = 0 ; i < Items.Length ; i++ )
  {
    if ( Items \[ i \] != null )
    {
      thread \[ i \] = new System.Threading.Thread ( Items \[ i \] ) ;
      thread \[ i \].Start() ;
    }
  }

  for ( int i = 0 ; i < thread.Length ; i++ )
  {
    if ( thread \[ i \] != null )
    {
      thread \[ i \].Join() ;
    }
  }

  return ;
}

(Untested)

Gary Wheeler

The only reason I can imagine doing something like that is if there is a requirement that GenerateTimeLineImage() execute on a non-UI thread, and yet be synchronized to the UI thread that requests the operation. There are plenty of the opposite circumstance (requesting a UI operation from a worker thread), but this sounds strange.

Software Zen: delete this;

PIEBALDconsult

Or perhaps to protect the UI thread from Exceptions? :confused:

AspDotNetDev

Or perhaps there is another thread that somehow manages these extra threads (e.g., abort them if they take too long).

Thou mewling ill-breeding pignut!

BobJanova

That's even worse since it's effectively a really obfuscated empty catch block!

BobJanova

The Thread object is local so I don't think this can be happening.

Member 7679313

Call me a nice guy who generally gives people the benefit of the doubt, but this looks like code where someone INTENDED for some operations to occur on separate threads asynchronously, put some of the plumbing in, put a thread.join in for synchronous debugging, and never got back to it. You are correct in that the code would be more performant without creating the additional thread as they presently are, but the context of the usage is also important in determining your predecessor's intent. If GenerateTimeLineImage() is a void function that is basically a "fire and forget" service call, commenting out m_thread.Join() might increase application performance, and by quite a bit. I'll tell you that there is no hidden magic, but you haven't provided enough context for me to be as condemning as some other folks here.

jschell

PIEBALDconsult wrote:

Or perhaps to protect the UI thread from Exceptions?

I suspect that would probably be a misunderstanding of exceptions. If the thread code doesn't catch exceptions then it will terminate the application, thus certainly impacting the UI. And if it does catch exceptions then just the same as wrapping the method call in a try/catch.