Multithreading done "right"

[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.

[Lost User]

Member 7679313 wrote:

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.

Ok, I call you a nice guy :) He did write that he found this in serveral places in the code, wich makes some kind of accident less likely. I would just take a look at some other projects he worked on and if something like that also appears there, then there is little room left for doubt.

At least artificial intelligence already is superior to natural stupidity