From: Peter Duniho on
Hector Santos wrote:
> [...] The fact is you
> got a BUG and you should be MAN enough to admit.

You can claim there's a bug until you're blue in the face, it still
won't make it true. You have not, and cannot, demonstrate the code I
posted breaking in the context in which I specifically stated it was
intended to be used.

Unfortunately, your ego prevents you from actually _reading_ what has
been written. Which is especially ironic, considering your "you should
be MAN enough" comment.

Look at the facts. Not your ego-driven bias.

Pete
From: Adam Clauss on
On 6/3/2010 6:10 AM, Hector Santos wrote:
> And that was simply by eyeballing it and then verifying what would
> easly BREAK IT - and yet you went on a MAJOR ASSUMPTION that I didn't
> try your code, and you went on with that incorrect assumption. The
> fact is you got a BUG and you should be MAN enough to admit.
>
I may regret this, but I'm going to stick my neck into this, as honestly
I don't think either one of you is completely understanding the other
:) Hopefully I pull my neck back before I get it chopped off!

Hector - I think Peter's point is that yes, you came up with a situation
where Peter's code does not work. However, the situation is a case
where it was NEVER intended to work. You created the "bug" in a console
application - the BackgroundWorker (to my knowledge) is designed solely
to function within a Windows Forms application (maybe WPF?? Not sure on
that one - I'll just stick with Windows Forms since you used
Application.DoEvents() in your initial example).

The threading behaves VERY differently between the two environments. In
a Windows Forms environment, as your original code snippet suggests you
are with the Application.DoEvents call, the
BackgroundWorker.RunWorkerCompleted event is guaranteed to ONLY execute
on the UI thread, and thus by definition can only be executed one at a
time. Since, at the end of Peter's post, he indicated that his sample
assumes "RunWorkers" is ALSO on the UI thread, that means that
RunWorkers itself will have to complete prior to any invocations of the
RunWorkerCompleted occurring. So, the only place "count" ever gets
modified (or accessed, assuming DoWork does not also access it), is from
the UI thread itself, which ensures there is no
race-condition/contention issues. That negates the need to pull out a
separate variable "threadCount" since "count" cannot, given that code
sample, be modified prematurely.

If RunWorkerCompleted is executed off the UI thread (or, ultimately I
guess more importantly the thread which executed RunWorkers), then yes,
the concerns you brought up are valid. However, the entire purpose of
BackgroundWorker is to get around that problem in a UI environment.

If you take the code sample outside the context they are given, sure,
they can break - but that is true about most (all?) code :)

-Adam

From: Peter Duniho on
Adam Clauss wrote:
> On 6/3/2010 6:10 AM, Hector Santos wrote:
>> And that was simply by eyeballing it and then verifying what would
>> easly BREAK IT - and yet you went on a MAJOR ASSUMPTION that I didn't
>> try your code, and you went on with that incorrect assumption. The
>> fact is you got a BUG and you should be MAN enough to admit.
>>
> I may regret this, but I'm going to stick my neck into this, as honestly
> I don't think either one of you is completely understanding the other
> :) Hopefully I pull my neck back before I get it chopped off!

I've no plans to do so. :)

Still, you promised to point out how I've misunderstood Hector, but I
didn't see anything in your post to point that out. If I have, an
independent observer such as yourself might have just the insight that
would be useful to me to understand that.

As for what you did write…

> Hector - I think Peter's point is that yes, you came up with a situation
> where Peter's code does not work. However, the situation is a case
> where it was NEVER intended to work. You created the "bug" in a console
> application - the BackgroundWorker (to my knowledge) is designed solely
> to function within a Windows Forms application (maybe WPF?? Not sure on
> that one - I'll just stick with Windows Forms since you used
> Application.DoEvents() in your initial example).

Yes, you have understood my point correctly. The original post and the
use of BackgroundWorker all suggest a GUI context, and thus the capacity
to take advantage of BackgroundWorker's features.

It's useful to note that BackgroundWorker will work in _any_ thread
where there's a marshaling SynchronizationContext. That is, the default
SynchronizationContext doesn't actually marshal delegate invocations
from one thread to another, but the SynchronizationContext found in a
thread where Application.Run is running, or a WPF dispatcher is running,
will do that.

So by default, BackgroundWorker provides the full functionality of its
features in either a Forms or WPF application, provided it's created in
a GUI thread in that application. Furthermore, it is possible to write
one's own SynchronizationContext class that works in other contexts.
The important thing is that a SynchronizationContext is available for
BackgroundWorker to use that will marshal delegate invocations onto the
thread that created the BackgroundWorker. It doesn't have to be a
Forms, or even WPF, application.

> The threading behaves VERY differently between the two environments. In
> a Windows Forms environment, as your original code snippet suggests you
> are with the Application.DoEvents call, the
> BackgroundWorker.RunWorkerCompleted event is guaranteed to ONLY execute
> on the UI thread, and thus by definition can only be executed one at a
> time. Since, at the end of Peter's post, he indicated that his sample
> assumes "RunWorkers" is ALSO on the UI thread, that means that
> RunWorkers itself will have to complete prior to any invocations of the
> RunWorkerCompleted occurring. So, the only place "count" ever gets
> modified (or accessed, assuming DoWork does not also access it), is from
> the UI thread itself, which ensures there is no
> race-condition/contention issues. That negates the need to pull out a
> separate variable "threadCount" since "count" cannot, given that code
> sample, be modified prematurely.

That is all exactly correct.

> If RunWorkerCompleted is executed off the UI thread (or, ultimately I
> guess more importantly the thread which executed RunWorkers), then yes,
> the concerns you brought up are valid. However, the entire purpose of
> BackgroundWorker is to get around that problem in a UI environment.

Exactly. There's hardly any point in using BackgroundWorker where the
BackgroundWorker instance isn't created in a thread where the
SynchronizationContext stuff will work. And since the original code
example called Application.DoEvents(), it's safe to assume that the
context is in fact a Forms application where the BackgroundWorker
instances are created in the GUI thread (there's no need to call
Application.DoEvents() if you're not executing in the main GUI thread
and blocking it).

Finally, see below for a WPF example that incorporates the code I posted
in a full application.

To Hector: you will find that it is not possible to cause the code to
fail to correctly manage the value of the "count" variable without
completely changing the nature of the code example (you can break any
code if you just ignore the preconditions set for the use of that code).
In particular, it does not matter how many workers you create or what
the DoWork event handler does, the code will work fine.

> If you take the code sample outside the context they are given, sure,
> they can break - but that is true about most (all?) code :)

Exactly.

Pete

MainWindow.xaml:
<Window x:Class="TestThreadCounter.MainWindow"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
Title="MainWindow" Height="350" Width="525">
<Grid>
<Button Content="Run Threads" Height="23"
HorizontalAlignment="Left" Margin="12,12,0,0" Name="button1"
VerticalAlignment="Top" Width="114" Click="button1_Click" />
</Grid>
</Window>


MainWindow.xaml.cs:
using System;
using System.Windows;
using System.ComponentModel;

namespace TestThreadCounter
{
/// <summary>
/// Interaction logic for MainWindow.xaml
/// </summary>
public partial class MainWindow : Window
{
public MainWindow()
{
InitializeComponent();
}

private void button1_Click(object sender, RoutedEventArgs e)
{
RunWorkers(100);
}

private void RunWorkers(int count)
{
string originalText = button1.Content.ToString();

button1.IsEnabled = false;

for (int i = 0; i < count; i++)
{
int delay = i * 25;
BackgroundWorker worker = new BackgroundWorker();

worker.DoWork += (sender, e) =>
{
DateTime dtEnd = DateTime.Now + new TimeSpan(0,
0, 0, 0, delay);

while (DateTime.Now < dtEnd)
{
//Thread.Sleep(1);
}
};

worker.RunWorkerCompleted += (sender, e) =>
{
if (--count > 0)
{
UpdateButton(count);
}
else
{
AllWorkersDone(originalText);
}
};

worker.RunWorkerAsync();
UpdateButton(i);
}
}

private void UpdateButton(int count)
{
button1.Content = string.Format("{0} worker{1} left",
count.ToString(), count > 1 ? "s" : "");
}

private void AllWorkersDone(string originalText)
{
button1.Content = originalText;
button1.IsEnabled = true;
}
}
}
From: Adam Clauss on
On 6/3/2010 10:35 PM, Peter Duniho wrote:
> Still, you promised to point out how I've misunderstood Hector, but I
> didn't see anything in your post to point that out. If I have, an
> independent observer such as yourself might have just the insight that
> would be useful to me to understand that.

Hehe, fair enough - I did forget to go down that path. Got down into
the technical details of the issue and hit send when I was done with them :)

Not so much that you were misunderstanding him, but maybe not making it
as easy for him to understand you. I had to jump back and forth quite a
bit between posts trying to connect all the dots. Part of that may
simply be the inline quoting response. The various pieces of important
and relevant information get spread out across several different
places. That was what I hoped to accomplish by my "block" reply -
consolidating all of the information into a single place.

Normally I think the inline responses are a good way to address
different points of a topic, but this time it made it harder for some
reason...

Just my thoughts...

-Adam
From: Mihajlo Cvetanović on
On 6/3/2010 5:25 PM, Peter Duniho wrote:
> Hector Santos wrote:
>> [...] The fact is you
>> got a BUG and you should be MAN enough to admit.
>
> You can claim there's a bug until you're blue in the face, it still
> won't make it true. You have not, and cannot, demonstrate the code I
> posted breaking in the context in which I specifically stated it was
> intended to be used.

Hector believes this would be a demonstration: in the first iteration of
the main loop first thread is created, its function set, and spawned to
run. BUT, when its time for second iteration first thread might already
be finished. This in turn decrements count, which reduces the number of
threads that would be spawned.

In C++ world this would be the case because RunWorkerCompleted would run
from spawned thread's context, but in C# world it will be run from main
thread's context, AFTER all threads are already spawned.

So, really, there's no bug here, AFAICS.