Multithreading done "right"

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

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

[BobJanova]

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

[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

[KP Lee]

If GenerateTimeLineImage is managing multiple threads, then the code makes sense. If not, then the coder read something, didn't understand it, but thought it was "cool", without understanding what the method was for.

[spencepk]

That explains everything :)

[Florin Jurcovici]

If there's version control in place, have a look - it might be that previous versions had some code between .Start() and .Join().