Reputation:
so this code takes value of incremented numbers stream with random starting number. If start1 value bigger then start2 I want display corresponding line with textbox, else another line.
Problem is I can't stop program until the given number of cycles is not satisfied. Button just hangs during implantation. I understand that the reason why this happens is a loop. Now I'm trying stop it with backgroundWorker, but I got same result, cancel button hangs same way.
Code which I want use with backgroundWorker located inside backgroundWorker1_ProgressChanged. I don't really quite understand what is happening here. Maybe you help me to figure out what I'm doing wrong:
using System;
using System.ComponentModel;
using System.Threading;
using System.Windows.Forms;
namespace XX_8_0
{
public partial class Form1 : Form
{
public Form1()
{
InitializeComponent();
backgroundWorker1.WorkerReportsProgress = true;
backgroundWorker1.WorkerSupportsCancellation = true;
}
private void startAsyncButton_Click(object sender, EventArgs e)
{
if (backgroundWorker1.IsBusy != true)
{
backgroundWorker1.RunWorkerAsync();
}
}
private void cancelAsyncButton_Click(object sender, EventArgs e)
{
if (backgroundWorker1.WorkerSupportsCancellation == true)
{
backgroundWorker1.CancelAsync();
}
}
private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
{
BackgroundWorker worker = sender as BackgroundWorker;
for (int i = 1; i <= 10; i++)
{
if (worker.CancellationPending == true)
{
e.Cancel = true;
break;
}
else
{
System.Threading.Thread.Sleep(500);
worker.ReportProgress(i * 10);
}
}
}
private void backgroundWorker1_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
var random = new Random();
var start1 = random.Next(0, 100);
var start2 = random.Next(0, 100);
var incrementor1 = start1 > 50 ? -1 : 1;
var incrementor2 = start2 > 50 ? -1 : 1;
var cV1 = start1;
var cV2 = start2;
for (var i = 0; i < 1000; i++)
{
if (cV1 == 101) incrementor1 = -1;
if (cV1 == 0) incrementor1 = 1;
if (cV2 == 101) incrementor2 = -1;
if (cV2 == 0) incrementor2 = 1;
if (cV1 > cV2)
{
textBox1.AppendText("ID: (" + i + ") CV1: (1): [" + cV1 + "] CV2: (0) [" + cV2 + "]\n");
}
else
{
textBox1.AppendText("ID: (" + i + ") CV1: (0): [" + cV1 + "] CV2: (1) [" + cV2 + "]\n");
}
cV1 += incrementor1;
cV2 += incrementor2;
}
}
private void backgroundWorker1_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
if (e.Cancelled == true)
{
resultLabel.Text = "Canceled!";
}
else if (e.Error != null)
{
resultLabel.Text = "Error: " + e.Error.Message;
}
else
{
resultLabel.Text = "Done!";
}
}
}
}
Upvotes: 1
Views: 93
Reputation:
Note: I think your example is perhaps not really a good example of what BackgroundWorker
can do. Essentially your code just sleeps a bit then reports progress, hardly useful in another thread.
Your backgroundWorker1_ProgressChanged
is quite heavy with adding 1000 textBox1.AppendText()
and thus could take some time.
Button just hangs during implantation. I understand that the reason why this happens is a loop.
When you consider backgroundWorker1_ProgressChanged
executes on the UI thread, no matter what number of clicks you make on cancel won't be processed until backgroundWorker1_ProgressChanged
returns. All UI processing has to be done by the UI thread. It should be pointed out that the background worker thread is also suspended during this time and unfortunately it is the only part of the code that tests for cancellation. Even if ReportProgress
were asynchonous, you still require the UI thread to process the cancel button click event and mark the work as CancellationPending
.
I suggest you don't report as much in one go in backgroundWorker1_ProgressChanged
or perhaps consider reporting adding items to the textBox1
as batches.
That way you won’t flood your message pump and your application will be more responsive.
Change your backgroundWorker1_ProgressChanged(object sender, ProgressChangedEventArgs e)
to use batches as:
var builder = new StringBuilder();
for (var i = 0; i < 1000; i++)
{
if (cV1 == 101) incrementor1 = -1;
if (cV1 == 0) incrementor1 = 1;
if (cV2 == 101) incrementor2 = -1;
if (cV2 == 0) incrementor2 = 1;
if (cV1 > cV2)
{
builder.Append("ID: (" + i + ") CV1: (1): [" + cV1 + "] CV2: (0) [" + cV2 + "]\n");
}
else
{
builder.Append("ID: (" + i + ") CV1: (0): [" + cV1 + "] CV2: (1) [" + cV2 + "]\n");
}
cV1 += incrementor1;
cV2 += incrementor2;
}
textbox1.AppendText(builder.ToString());
Upvotes: 4
Reputation: 2624
Since your ProgressChanged
handler is very busy updating the UI about 1000 times per ReportProgress
call, it will use all UI thread CPU time and that thread will never be able to process Cancel button since it is already way too busy.
Given that you use a BackgroundWorker
, you should really move most of ProgressChanged
in the DoWork
handler and remove that sleep in the background thread.
The way, you use the BackgroundWorker
does not make any sense. The UI thread should do as little as possible and the background thread should as much as possible. And it does not make sense to sleep in the background thread in your case as the background work does not depend on time elapsed.
Once the code has been moved, you need to send the data back to the UI thread. But this is very easy since there in an overload of ReportProgress
that allow to pass any .NET object to the UI thread (assuming the background worker is created from the UI thread which is generally the case). On the other side, you simple cast the user data to the appropriate type and use that information to append appropriate text to the text box.
With improved code, you should be able to process that data much faster. By the way a StringBuilder
should also be used to build the string. In fact, it should have a dramatic impact on UI performance since you update the UI text 1000 time less often.
Upvotes: 0