-
[RESOLVED] How to UseWaitCursor correctly
I have a Windows Forms app that does a lengthy calculation in a button's click handler. So I set the form's UseWaitCursor property to true when I start the calculation and reset it to false when I'm done.
Sounds simple, but it doesn't work like it should. The wait cursor doesn't appear instantly, instead it takes time until I click on an arbitrary button (clicking on the form or other controls doesn't seem to have that effect), and even that doesn't always work the first time. :ehh: Does anybody have a clue on how I could get rid of that annoying behaviour?
TIA
-
Re: How to UseWaitCursor correctly
Ok, now that I have also encountered repainting problem with that app after writing my OP, I tried adding a call to Application::DoEvents() inside the loop. Now the wait cursor is shown correctly and the repainting problem is gone too. :) But now the normal cursor isn't always restored after the loop has finished. Sometimes it waits to restore the original cursor until I move the mouse. Now what then is this, and how can I get rid of it? :confused:
Furthermore, I'm concerned about calling DoEvents(). I have never used it in VBA due to the warning about re-entry situations in the docs, and the MSDN article on the CLR function contains a waring that's quite similar. The suggested remedy is to move the time-consuming computation to a worker thread. This is no black magic to me either, because I have already done this in a quite similar situation in an MFC app. But for exatly that reason I know that it would considerably complicate things. Is there any less complicated way to make it a little safer?
-
Re: How to UseWaitCursor correctly
DoEvents is easy method allowing to keep UI responsive while long calculation is done. To have consistent results, you need to disable every control, possibly the whole form, while the loop is running, to prevent unexpected results, like closing the form or button click. Another way is to ignore any user command when long calculation is active, this can be done by using simple boolean flag. Of course, multithreading is the real thing, if you have a reason to use multithreading, do it and leave DoEvents to VB6 programmers.
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Alex F
DoEvents is easy method allowing to keep UI responsive while long calculation is done. To have consistent results, you need to disable every control, possibly the whole form, while the loop is running, to prevent unexpected results, like closing the form or button click.
Would disabling the form automatically affect all the child controls or would I have to do that explicitly for every individual control?
Quote:
Another way is to ignore any user command when long calculation is active, this can be done by using simple boolean flag.
This might be simpler if I can do that in a single, central place. Would a message filter (Application::AddMessageFiler()) be appropriate? And how would I distiguish messages related to user actions from those coming from the OS like WM_PAINT?
Quote:
Of course, multithreading is the real thing, if you have a reason to use multithreading, do it and leave DoEvents to VB6 programmers.
Ok, maybe I'll finally pick that option once I have some spare time. This is only an experimental app anyway (although it has grown up to a really usable tool in the meantime), but OTOH, why not use these experimental apps to learn the real thing? :cool:
But well, from the user's point of view, this isn't meant to be a multi-threaded app at all. Do I really gain something by doing the work in some detached thread while disabling all the controls anyway?
-
Re: How to UseWaitCursor correctly
Well-designed multithreaded program for this case (synchronous long loop) must show some kind of progress dialog with "Stop" button (if this necessary). Program with DoEvents should also allow to stop the process. The simplest way in both cases is to show some modal dialog with progress bar. If Stop is an option, it should react on the Stop button, and ignore any other commands. Without Stop, this dialog must ignore any user activity. In both cases, Paint messages are handled and application must redraw itself.
Modal progress dialog automatically prevents any user interaction with its parent window, this solves the problem of unexpected commands.
-
1 Attachment(s)
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Alex F
Modal progress dialog automatically prevents any user interaction with its parent window, this solves the problem of unexpected commands.
This looks like it would solve the re-entry problem in an elegant manner and also would be another nice feature upgrade. :cool:
I have added a progress dialog to the app that you can see in the attached image. Is that design "canonical"? It doesn't have a Stop button so far as I considered implementing it too complicated for now, and I suppose that I also would have to considerably change the way the stop button works when I change the way the progress dialog works.
You see that the progress dialog still has an X button. I could easily get rid of all the others but I couldn't figure out how to get rid of this one. I suppose that deactivating (not hiding) the progress form would render that button irresponsive, but then the form would look disabled and it might have other adverse effects.
Currently, the progress form is displayed using the Show() method from within the processing function that is a member of the main form class and gets called from a button's click handler. I suppose that already the fact that the Show() method returns instantly means that the dialog is not modal. I simply couldn't find a method like MFC's CDialog::DoModal() in the Form class. But OTOH the main form's title bar is displayed in the disabled state which could mean that the form is irresponsive to user activities, isn't it? At least repainting is working like it should.
Ah, and I noticed that the setting of UseWaitCursor has no effect on the cursor when it is in the form's non-client area. Is that an indication of some problem?
Processing time required for a 731 MB file is increased by about 67% by the progress processing. The progress bar's Increment() method is called every 512 bytes (might be too often but this is the most natural update frequency for the current execution structure) and the progress form's Text property is updated only when needed, IOW the percentage number actually changes. This might be a lot, but waiting is less boring when I can watch the progress dialog, and thus my subjective impression is even an improvement in processing speed! :D
-
Re: How to UseWaitCursor correctly
You need to solve the basic problem of all apps that perform long running cpu intensive operations - that is, how to keep the UI responsive.
The quick and dirty, tightly coupled difficult to maintain way is to put the long running code into the UI and pepper the code with Application.DoEvent calls. (as you can tell, there isn't any bias in my previous response).
The proper way to do it where the user can start, stop, pause and resume the long running operation is to put the long running code into a worker thread (like BackgroundWorker) and update the ui with InvokeRequired.
All you're updating, cursor, buttons failing to disable, etc issues are due to the fact that the long running operation doesn't allow the main UI thread to pump messages which prevents the operations from running that allow the cursor to change, the button to disable, or the form to repaint. These are classic symptoms when long running operations are put into the UI thread.
Moving the long running operations into a worker thread requires more work to learn and implement, but once you learn how, it gets pretty easy. It also results in loosely coupled code that is easier to maintain. The code typically ends up being easier to unit test because the code performing the actual work isn't jammed in with the UI code.
-
Re: How to UseWaitCursor correctly
The required method is ShowDialog. To prevent user to close it, handle FormClosing event.
To stop calculation, if necessary, set some boolean flag when user clicks the Stop button, and test for this flag after every call to DoEvents in the calculation loop.
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Arjay
The quick and dirty, tightly coupled difficult to maintain way is to put the long running code into the UI and pepper the code with Application.DoEvent calls. (as you can tell, there isn't any bias in my previous response).
This is the way it is implemented right now. However, there is only a single call to Application::DoEvents() right at the end of the outer one of the two nested core loops, and that seems to be enough for now.
Quote:
All you're updating, cursor, buttons failing to disable, etc issues are due to the fact that the long running operation doesn't allow the main UI thread to pump messages which prevents the operations from running that allow the cursor to change, the button to disable, or the form to repaint. These are classic symptoms when long running operations are put into the UI thread.
All these symptoms are gone now, but I'm still not completely satisfied yet. ;)
Quote:
The proper way to do it where the user can start, stop, pause and resume the long running operation is to put the long running code into a worker thread (like BackgroundWorker) and update the ui with InvokeRequired.
[...]
Moving the long running operations into a worker thread requires more work to learn and implement, but once you learn how, it gets pretty easy. It also results in loosely coupled code that is easier to maintain. The code typically ends up being easier to unit test because the code performing the actual work isn't jammed in with the UI code.
This is the way I want it to look when it's finished. I already had a brief look at the BackgroundWorker docs, but that's a lot stuff to read. I'm almost sure I'll come back with some more questions once I've finished reading (and maybe started implementing). ;)
The Stop button, BTW, is already there, although implemented the quick and dirty way. (See below.) I don't think there really is a need for Pause and Resume features in this simple app, however. (I can't even imagine what users should do with my app during a pause... :ehh:)
Quote:
Originally Posted by Alex F
The required method is ShowDialog.
Uh, how could that slip through!? It's right next to the Show() method I already used in the members list! :o And I have already used ShowDialog() with the OpenFileDialog class and could well have extrapolated from there.
In fact, I already found ShowDialog() when I was working on another app, where I needed to use a form in the classic dialog box manner: to acquire user input. And that really could not have been done without that method in a reasonable way... But thank you for the pointer anyway, of course you couldn't have known that. :)
Quote:
To prevent user to close it, handle FormClosing event.
I implemented it that way (this time after reading your tip, thanks), but it looks like a hack to me, as the button isn't really disabled, it just has no effect. But finally...
Quote:
To stop calculation, if necessary, set some boolean flag when user clicks the Stop button, and test for this flag after every call to DoEvents in the calculation loop.
This is the way it is implemented right now. Works like a charm! :) And just by the way it takes away my concern about blocking the X button: I simply implemented the same semantics that the "literal" Cancel button has for the X button. Or is that non-canonical hack-style too?
I really appreciate the assistance I get from both of you. :) Too bad I can't give you a positive rating yet again...
-
Re: How to UseWaitCursor correctly
Just side note: user may be surprised when some operation is stopped after pressing the Close button on the progress dialog.
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Alex F
Just side note: user may be surprised when some operation is stopped after pressing the Close button on the progress dialog.
Actually, I was uncertain about that too. But how should I hande it then? Simply continue processing without the progress display (and the oppotunity to cancel the operation after that)?
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Eri523
Actually, I was uncertain about that too. But how should I hande it then? Simply continue processing without the progress display (and the oppotunity to cancel the operation after that)?
Just add a Cancel button to the progress dialog. If the user presses the cancel button or the close 'X' button, ask the user of they wish to cancel the operation. If the operation is such that once started it can't be cancelled, then don't show a cancel button and disable the 'X' button.
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Arjay
Just add a Cancel button to the progress dialog.
It already has one. :)
Quote:
If the user presses the cancel button or the close 'X' button, ask the user of they wish to cancel the operation.
Nice idea, but I think I'll only implement that confirmation for the X button. Asking that after the user pressed the explicitly labeled Cancel button too much looks like some "Are you really sure?" kinda question to me. ;)
I'm afraid there is no variant of the standard message box with a Cancel and a Continue button. Using the Ok/Cancel variant and giving the Cancel button the meaning cancel the cancellation might be puzzling too. :D I think I'll settle for the Yes/No variant, that's pretty neutral.
Quote:
If the operation is such that once started it can't be cancelled, then don't show a cancel button and disable the 'X' button.
The operation can be cancelled eaisly, that's no problem.
Disabling the X button exactly was the problem I had with the variant of the progress dialog that didn't support cancellation: I couldn't find a way to do that. That's why Alex F suggested aborting the closing process in the FormClosing() handler.
-
Re: How to UseWaitCursor correctly
The X button issue is solved now. :) I found the solution "by accident" among the results of a forum search I did for a completely different reason.
The solution is: Set the form's ControlBox property to false. The docs on that property say it's for disabling the form's system menu, which is gone anyway after setting FormBorderStyle to FixedDialog, so I never considered touching it. But it also hides the X button as some kind of side effect. :cool:
Now it's time to tackle implementing the background worker...
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Eri523
It already has one. :)
Nice idea, but I think I'll only implement that confirmation for the X button. Asking that after the user pressed the explicitly labeled Cancel button too much looks like some "Are you really sure?" kinda question to me. ;)
I'm afraid there is no variant of the standard message box with a Cancel and a Continue button. Using the Ok/Cancel variant and giving the Cancel button the meaning cancel the cancellation might be puzzling too. :D I think I'll settle for the Yes/No variant, that's pretty neutral.
The operation can be cancelled eaisly, that's no problem.
Disabling the X button exactly was the problem I had with the variant of the progress dialog that didn't support cancellation: I couldn't find a way to do that. That's why Alex F suggested aborting the closing process in the FormClosing() handler.
The screen shot doesn't already have a cancel button. What you have is a close button (i.e the 'X' button). You can disable the 'X' button. Not sure how off the top of my head, but I'll look in google in a minute.
The user prompt to verify canceling isn't like a "are you really sure?". It's pretty much the standard when cancelling a long running operation in this manner. If you don't have it, your users will be pissed when they thought the 'X' would merely hide the progress dialog.
Btw, this isn't rocket science with regard to the standard message box buttons. Just use Yes/No and phrase the question accordingly. Something like "This will cancel the long running critical operation. You will have to rerun the long running critical operation again. Do you wish to cancel the long running critical operation?" "Yes" "No".
-
1 Attachment(s)
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Arjay
The screen shot doesn't already have a cancel button. What you have is a close button (i.e the 'X' button).
Ah, yes, of course, sorry... :o The screen shot you are referring to is a few versions of the app back. I have attached a current one to this post. (I hope this doesn't look like image overkill Ã* la firoz.raj now. ;))
Quote:
You can disable the 'X' button. Not sure how off the top of my head, but I'll look in google in a minute.
Sorry again: I just posted that I found the solution for that issue (including the solution itself) 11 minutes before your post I'm just replying to. Looks like we encountered some kind of synchronisation problem here. Maybe we should use some kind of semaphore or the like? :) I hope you read this one in time before you waste any time.
Quote:
The user prompt to verify canceling isn't like a "are you really sure?". It's pretty much the standard when cancelling a long running operation in this manner. If you don't have it, your users will be pissed when they thought the 'X' would merely hide the progress dialog.
I 100% agree when it comes to the X button. But is this really appropriate (or even required) for a button explicitly labelled "Cancel"? :ehh:
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Eri523
I 100% agree when it comes to the X button. But is this really appropriate (or even required) for a button explicitly labelled "Cancel"? :ehh:
It depends on the 'severity' of what happens if the user cancels. If the user can restart the operation easily enough and the operation doesn't take much time, then you don't need the "are you sure" check. On the other hand, if the operation is destructive or takes a lot of time to run, then you should have one.
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Arjay
It depends on the 'severity' of what happens if the user cancels. If the user can restart the operation easily enough and the operation doesn't take much time, then you don't need the "are you sure" check. On the other hand, if the operation is destructive or takes a lot of time to run, then you should have one.
Ok, then I don't think I need this extra check. Processing can be restarted easily by just clicking a button and picking a file in a file open dialog. In the current version, processing of the 731 MB test file takes slightly below 30 seconds. It is faster now again, which is a side-effect of changing from synchronous to asynchronous file processing. But I don't think this is because the worker thread is that amazingly fast as-such, rather it is because I reduced the frequency of progress updates. And as I think I'm limited to a maximum file size of 2 GB anyway, total processing time can't grow that much.
As a contrast, BTW, processing time for a small 6 kB file grew from about 15 ms to 250 ms. :ehh:
I started changing the program from synchroous to asynchronous processing more or less right after writing my post #14. I had to go some distance before I got to a state that I could compile and run, but than it worked almost instantly. :)
... as long as I didn't click the Cancel button. :rolleyes: If I did that, I ran into a debug assertion. At least it was one that I placed in the program as a precaution myself. Secifically, it was the one on bwDoCount->IsBusy in Form1::DoCount(FileStream ^) (see below).
The current version does an assertion failure on the line just above that, in the assertion on bEwhSignaled.
EDIT: One really important thing I forgot to mention: To my big surprise, Form1::bwDoCount_RunWorkerCompleted() appears to not even get called at all when I click the Cancel button. Or, and that might be even a bit stranger, it actually is called, but the breakpoint I placed there has no effect. :ehh:
Actually I had to do some brain twisting to get the combination of the worker thread and the modal progress dialog to work at all, as I obviously can't do any work in the GUI thread as long as the progress dialog is open. Maybe the solution I came up with is a bit screwed up, though I don't hope so. ;)
I have been struggling with that a whole lot of time since then, and I can hardly believe that I started this just last sunday. It feels like ages... :sick:
I did a lot of fiddling around with the code to no avail. Finally, I reconstructed a state that is close to the initial one and of which I think it's a reasonable point to start over.
As I have run out of ideas now, I hope for some help from you guys around here. Below are the functions that I think are relevant. All variables that are not local are members of the respective classes. I hope they're self-explanatory enough, as I won't include all the declarations for now.
These are the functions involved in the main form class (Form1):
Code:
System::Void Form1::DoCountWorker(FileStream ^fsInfile,BackgroundWorker ^bw,DoWorkEventArgs ^e)
{
BinaryReader brInfile(fsInfile);
const int nBufferSize=512;
array<Byte> ^abBuffer;
int nPercent=0,nLastPercent=-1;
do {
abBuffer=brInfile.ReadBytes(nBufferSize);
for (int i=0;i<abBuffer->Length;++i)
++aulCounts[abBuffer[i]];
if ((nPercent=(100*fsInfile->Position)/fsInfile->Length)!=nLastPercent) {
bw->ReportProgress(nPercent);
nLastPercent=nPercent;
}
} while (!bw->CancellationPending && abBuffer->Length==nBufferSize);
brInfile.Close();
if (bw->CancellationPending) e->Cancel=true;
}
System::Void Form1::DoCount(FileStream ^fsInfile)
{
progress=gcnew Progress;
ewhWorkerReallyFinished=gcnew EventWaitHandle(false,EventResetMode::AutoReset);
UseWaitCursor=true;
progress->bwParentWorker=bwDoCount; // To enable calling back to the BackgroundWorker object
#ifdef _DEBUG
Diagnostics::Stopwatch stw;
stw.Start();
#endif
for (int i=0;i<256;++i) aulCounts[i]=0;
#ifdef _DEBUG
Diagnostics::Debug::Assert(!bwDoCount->IsBusy,"Background worker already busy when going to start it");
#endif
bwDoCount->RunWorkerAsync(fsInfile);
System::Windows::Forms::DialogResult dlgr=progress->ShowDialog(this);
bool bEwhSignaled=ewhWorkerReallyFinished->WaitOne(60000); // To be really sure!
#ifdef _DEBUG
Diagnostics::Debug::Assert(bEwhSignaled,"Wait for worker shut-down timed out");
Diagnostics::Debug::Assert(!bwDoCount->IsBusy,"Background worker still running after cancellation");
#endif
fsInfile->Close();
if (!bCountCancelled) {
chart1->Titles[0]->Text=String::Concat("Byte frequency histogram of ",fsInfile->Name);
Series ^serTarget=chart1->Series["Byte frequencies"];
serTarget->Points->Clear();
for (int i=0;i<256;++i) serTarget->Points->AddXY(i,aulCounts[i]);
}
#ifdef _DEBUG
stw.Stop();
Diagnostics::Debug::WriteLine("Elapsed time: {0} ms",stw.ElapsedMilliseconds);
#endif
UseWaitCursor=false;
ewhWorkerReallyFinished=nullptr;
progress=nullptr;
}
System::Void Form1::bwDoCount_DoWork(System::Object^ sender, System::ComponentModel::DoWorkEventArgs^ e)
{
DoCountWorker(safe_cast<FileStream ^>(e->Argument),dynamic_cast<BackgroundWorker ^>(sender),e);
}
System::Void Form1::bwDoCount_ProgressChanged(System::Object^ sender, System::ComponentModel::ProgressChangedEventArgs^ e)
{
progress->Text=String::Format("Processing file... {0}%",e->ProgressPercentage);
progress->progressBar1->Value=e->ProgressPercentage;
}
System::Void Form1::bwDoCount_RunWorkerCompleted(System::Object^ sender, System::ComponentModel::RunWorkerCompletedEventArgs^ e)
{
if (e->Error) MessageBox::Show(this,e->Error->Message,"Error during file processing",MessageBoxButtons::OK,MessageBoxIcon::Error);
bCountCancelled=e->Cancelled;
if (progress->ContainsFocus) progress->Close(); // Progress dialog is still open: close it
ewhWorkerReallyFinished->Set();
}
I hope this isn't too much code for now.
There are some variables that just get assigned a function return value and then are never used again. These are for inspecting the return values in the debugger.
The progress form class only contains a single function that I defined myself:
Code:
System::Void Progress::bnCancel_Click(System::Object^ sender, System::EventArgs^ e)
{
bwParentWorker->CancelAsync();
}
I can zip and upload the entire project as well, if someone should be interested in that.
TIA
-
Re: How to UseWaitCursor correctly
Well, in the meantime I've gathered some more information about this issue. (But they're really few as I'm still short of ideas.)
But first let me pick up the following statement from my own previous post:
Quote:
Originally Posted by
Eri523
EDIT: One really important thing I forgot to mention: To my big surprise, Form1::bwDoCount_RunWorkerCompleted() appears to not even get called at all when I click the Cancel button. Or, and that might be even a bit stranger, it actually is called, but the breakpoint I placed there has no effect. :ehh:
I edited it into the middle of the post, two hours after doing the post itself. Maybe some people who have read the original post short time after I did it missed this one, as I placed it right in the middle of that really long post. ... really my fault... :blush:
An update closely related to this one: Form1::bwDoCount_RunWorkerCompleted() actually is called if file processing is completed the normal way, i.e. I don't click the Cancel button. And this confirms, in addition, that the breakpoint I set in the ..._RunWorkerCpleted() handler is indeed effective. It only isn't called if I click the Cancel button.
In the Fibonacci sample from MSDN, the completion function is called in both cases, like I (and probably everyone else) had expected. But the MSDN sample does invocation of the background worker, progress display and cancellation handling all within a single form, and thus was not really that good a model for what I'm doing in my app. (Actually, this was the reason for me having to do a lot of initial thinking before writing my code.)
Another fact: Any of the BackgoundWorker event handlers (except ..._DoWork() of course) runs in the context of the thread owning the BackgroundWorker object, IOW the GUI thread. But that's exactly what I expected, and anyone who really knows about that subject would have known that even more likely.
A funny side note at the end: If the MSDN docs were right, the debugger of the Express Edition wouldn't even have a Threads window. Luckily, this is not the case. :)
Any ideas now?
TIA
-
Re: How to UseWaitCursor correctly
Zip up the project and include any data files. Also include what is ocurring and what you expect.
-
3 Attachment(s)
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Arjay
Zip up the project and include any data files. Also include what is ocurring and what you expect.
Thank you for your effort Arjay, both backward and forward in time. :)
I have attached the zipped project in question. I did a clean-up on the project before zipping it, so I hope it doesn't contain any files that are unwanted here. If it does inspite of this, please tell me their extensions and I'll add them to my clean-up settings.
I also added two screen shots. One is the outcome of processing the plain text file Test.txt that I attached as well, when I don't click the Cancel button. But that's not really the issue.
The other screen shot shows the assertion failure I get when I actually click the Cancel button (after some waiting of course). Obviously, I can't attach the 731 MB file that gives me enough processing time to do so. But you can try any large file you want: The actual contents of the file doesn't really matter.
Again: Really, really big TIA. I'm really out of ideas what might cause this misbehaviour.
EDIT: Removed outdated attached project file. A new one is attached to post #35.
-
Re: How to UseWaitCursor correctly
Ok, now that I'd waited 1+ week for getting additional insights from here without success, I resorted to a Google search for "BackgroundWorker RunWorkerCompleted not called". Although I didn't expect much from that because I interpreted the lack of resonance here as an indication for the problem not being really common, I found something (19.600 hits).
This taught me at least three things: 1) The problem is not really that uncommon. 2) It is not limited to C++/CLI. 3) It is not new: The search hits date back to at least 2007.
I found at least a partial solution at http://www.experts-exchange.com/Prog..._22354237.html. The user posting there, who was using VB .NET, first resorted to calling Application::DoEvents(). What I suppose to be the real solution was unfortunately blurred on that page and covered by a box suggesting to sign up. :(
However, I tried adding a single (!) call to Application::DoEvents() before the line waiting for ewhWorkerReallyFinished to get signaled. (Those who didn't download the entire zipped project may have a look at the code excerpt in post #18.) And suddenly it worked like expected!
Yes, I know that calling Application::DoEvents() is evil, and actually the introduction of the background worker was initially intended to remove just that. But it simply works... :o
So now, can someone tell me why it works now and maybe how I could solve the problem in a more elegant or correct way?
And there's another thing I've learned: People are always told on this site to first try a Google (or whatever) search. Maybe I myself should take that option into account more often... :o
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Eri523
And there's another thing I've learned: People are always told on this site to first try a Google (or whatever) search. Maybe I myself should take that option into account more often... :o
That should always be your first attempt at solving the problem. More often than not, you can find the answer yourself.
I haven't looked at your code because I don't have VS2010 handy right now. If adding a DoEvents call causes your program to work, whatever you're doing is simply causing the UI thread not to be able to pump messages.
-
Re: How to UseWaitCursor correctly
This thread is too long, and has too much code, I don't think that anyone really reads this code. However, the last post about DoEvents gives me some idea. There is classic deadlock situation with a worker thread. Consider working thread sending synchronous (Invoke) messages to the main thread. At some point main thread starts to wait for worker thread end in wait operation, this means, it becomes unresponsive and doesn't handle any messages, including Invoke. At the same time, worker thread calls Invoke: deadlock. DoEvents in the main thread solves the problem, handling Invoke call and releasing the worker thread.
This situation is described here: http://www.codeproject.com/KB/cs/workerthread.aspx
Use BeginInvoke instead of Invoke, and this solves the problem. Generally, asynchronous call must be always default choice vs synchronous.
Also, it is a good idea to create thread manually at least once, instead of using BackgroundWorker. This allows to understand what really happens in multithreaded program. BackgroundWorker hides important implementation details.
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Alex F
This thread is too long, and has too much code, I don't think that anyone really reads this code.
Well, I already was afraid of something like that some messages up the thread... :(
But I promise it will be marked resolved soon. Soon, however, not now... :p
Quote:
However, the last post about DoEvents gives me some idea. There is classic deadlock situation with a worker thread. [...]
Yes apparently this is exactly the problem I encountered. I didn't take into account that BackgroundWorker might use ordinary Windows messages (or is there something else that's handled by the message pump?) internally for its inter-thread communication.
Quote:
Use BeginInvoke instead of Invoke, and this solves the problem. [...]
I didn't call either of the two myself, but I strongly suspect that BackgroundWorker did it internally. I'll go and read up on these methods the next days to learn more about the mechanisms of multithreading in the CLR environment.
Quote:
Also, it is a good idea to create thread manually at least once, instead of using BackgroundWorker. This allows to understand what really happens in multithreaded program. BackgroundWorker hides important implementation details.
I'm afraid your'e pretty darn right! :o I already considered using Threading::Thread instead of BackgroundWorker while looking for solutions, mostly because the convenience of its Join() method. But then I would have had to rewrite significant amounts of code and lose some of the convenience offered by BackgroundWorker, and so I refrained from doing so. Maybe I would have found a solution much quicker if I hadn't... :)
Thanks for the link to codeproject.com. I have seen the sample code there uses Threading::Thread, so I will study it in detail soon.
And now another question to the entire audience, that leads back to the original title of that thread: While fighting the deadlock problem I had put aside the fact that now again setting the form's UseWaitCursor property to true near the beginning of Form1::DoCount(FileStream ^) apparently has no effect. (I do see a wait cursor, however, for a short period after closing the progress dialog and before repainting the Chart control.)
I found a discussion about this problem here at MSDN, but the solution of calling the evil Application::DoEvents() right after setting UseWaitCursor didn't help either. The alternative of using Form::Cursor::Current instead, that I think I found elsewhere on MSDN was no solution too. A lack of message processing doesn't seem to be the problem here, as repainting works like a charm. Any tips?
What I don't want to do is setting the wait cursor application-wide, as I don't want it for the progress dialog. But maybe that wouldn't work either...
TIA again
-
Re: How to UseWaitCursor correctly
Before starting the worker thread, just save off the current cursor (using the Form's Cursor property) and set the new cursor.
When you receive the thread complete (or abort method), restore the cursor by setting the form cursor to the one you saved off.
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Arjay
Before starting the worker thread, just save off the current cursor (using the Form's Cursor property) and set the new cursor.
When you receive the thread complete (or abort method), restore the cursor by setting the form cursor to the one you saved off.
I thought I'd already tried that, but to be sure I tried it again, following your advice as close as possible.
I replaced the two assignments to the form's UseWaitCursor property in the method Form1::DoCount(FileStream ^) by
Code:
System::Windows::Forms::Cursor ^csrSaved=Cursor->Current;
Cursor->Current=Cursors::WaitCursor;
and
Code:
Cursor->Current=csrSaved;
respectively.
(The majority of you who didn't download the entire project from the attachment to post #21 may refer to the code excerpt in post #18. There have been some changes to the project in the meantime, but I don't think they're relevant here.)
Unfortunately: No effect, regardless of whether I follow the first code snippet by a call to Application::DoEvents() or not. :sick:
Any clues about a possible cause for that?
TIA
-
Re: How to UseWaitCursor correctly
Not sure where you're putting the following code
Quote:
Originally Posted by
Eri523
Code:
System::Windows::Forms::Cursor ^csrSaved=Cursor->Current;
Cursor->Current=Cursors::WaitCursor;
and
Code:
Cursor->Current=csrSaved;
One problem you have is that csrSaved needs to be a class field, saved off in the DoCount and restored in the RunWorkerCompleted.
Code:
System::Void Form1::DoCount(FileStream ^fsInfile)
{
// save off the cursor
csrSaved = Cursor->Current;
// other code removed for clarity
}
System::Void Form1::bwDoCount_RunWorkerCompleted(System::Object^ sender, System::ComponentModel::RunWorkerCompletedEventArgs^ e)
{
// restore the original cursor
Cursor->Current = csrSaved;
// other code removed for clarity
}
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Eri523
I didn't take into account that BackgroundWorker might use ordinary Windows messages (or is there something else that's handled by the message pump?) internally for its inter-thread communication....
I didn't call either of the two myself, but I strongly suspect that BackgroundWorker did it internally.
BackgroundWorker doesn't send anything itself, only your own code. I remember that in the beginning of this question we were talking about progress dialog. How progress information is sent to the main thread?
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Arjay
One problem you have is that csrSaved needs to be a class field, saved off in the DoCount and restored in the RunWorkerCompleted.
This doesn't help either. Without wanting to doubt your expertise, I didn't put much hope into this change anyway: The problem is not restoring the original cursor, but getting the wait cursor displayed in the first place.
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Alex F
How progress information is sent to the main thread?
Like that:
Code:
System::Void Form1::DoCountWorker(FileStream ^fsInfile,BackgroundWorker ^bw,DoWorkEventArgs ^e)
{
BinaryReader brInfile(fsInfile);
const int nBufferSize=512;
array<Byte> ^abBuffer;
int nPercent=0,nLastPercent=-1;
do {
abBuffer=brInfile.ReadBytes(nBufferSize);
for (int i=0;i<abBuffer->Length;++i)
++aulCounts[abBuffer[i]];
if ((nPercent=(100*fsInfile->Position)/fsInfile->Length)!=nLastPercent) {
bw->ReportProgress(nPercent);
nLastPercent=nPercent;
}
} while (!bw->CancellationPending && abBuffer->Length==nBufferSize);
brInfile.Close();
if (bw->CancellationPending) e->Cancel=true;
}
System::Void Form1::bwDoCount_ProgressChanged(System::Object^ sender, System::ComponentModel::ProgressChangedEventArgs^ e)
{
progress->Text=String::Format("Processing file... {0}%",e->ProgressPercentage);
progress->progressBar1->Value=e->ProgressPercentage;
}
Where progress is the progress dialog form object.
By debugging I found out that the bwDoCount_ProgressChanged() event handler runs in the context of the GUI thread, so IMO inter-thread communication is done internally by BackgroundWorker.
But the progress display doesn't cause the deadlock anyway, the problem was only exposed when I clicked the Cancel button. Cancellation is handled by this simple event handler in the progress dialog's form class:
Code:
System::Void Progress::bnCancel_Click(System::Object^ sender, System::EventArgs^ e)
{
bwParentWorker->CancelAsync();
}
Where bwParentWorker is a tracking handle to the BackgroundWorker object that gets passed (i.e. assigned) from the main form object to the progress dialog's form object.
Closing the progress dialog in case I did not click the cancel button is donne by the completion handler (and works):
Code:
System::Void Form1::bwDoCount_RunWorkerCompleted(System::Object^ sender, System::ComponentModel::RunWorkerCompletedEventArgs^ e)
{
if (e->Error) MessageBox::Show(this,e->Error->Message,"Error during file processing",MessageBoxButtons::OK,MessageBoxIcon::Error);
bCountCancelled=e->Cancelled;
if (progress->Visible) progress->Close(); // Progress dialog is still open: close it
Cursor->Current=csrSaved; // This has just been added in response to Arjay's latest post
ewhWorkerReallyFinished->Set();
}
But, as described above, this handler doesn't even get called when I click Cancel.
-
Re: How to UseWaitCursor correctly
It is not clear from your post what is bw->ReportProgress(nPercent) and how it calls bwDoCount_ProgressChanged. Some code is missing to understand this.
Anyway, comment off ReportProgress line and try to reproduce the cancelling problem: what happens?
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Alex F
It is not clear from your post what is bw->ReportProgress(nPercent) [...].
Oops! :o I tried to reduce the amount of code posted. Maybe I reduced a bit too much...
bw is a BackgroundWorker ^, passed as a parameter to DoCountWorker() from the worker event handler:
Code:
System::Void Form1::bwDoCount_DoWork(System::Object^ sender, System::ComponentModel::DoWorkEventArgs^ e)
{
DoCountWorker(safe_cast<FileStream ^>(e->Argument),dynamic_cast<BackgroundWorker ^>(sender),e);
}
And the event handler in turn gets the handle from the BackgroundWorker object itself.
Thus, ReportProgress() is the BackgroundWorker method as described on MSDN and bwDoCount_ProgressChanged() is the event handler called by the BackgroundWorker as in response to my call to that method.
Quote:
Some code is missing to understand this.
As you can conclude from the above, I don't have that code either; MS has. :o
Quote:
Anyway, comment off ReportProgress line and try to reproduce the cancelling problem: what happens?
Interesting thing to try. Result: same problem as before (assertion failure as shown in the screenshot in post #21, fired from within Form1::DoCount(FileStream ^)). :(
-
Re: How to UseWaitCursor correctly
Is the attached project code up to date? If not, updated it and I'll take another look at it.
As compared to C#, working in managed C++ is a real PITA, so I want to be sure the latest is posted before I look at it again.
-
1 Attachment(s)
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Arjay
Is the attached project code up to date? If not, updated it and I'll take another look at it.
No. I'll attach an updated one to this post and remove the one from the previous post.
Please let me know if it contains any unwanted files, so I can adjust my clean-up settings.
As I already mentioned, the input file used for testing doesn't really matter. The Test.txt attached to the previous post was used to generate the screenshot, however.
TIA again for your efforts! :)
-
Re: How to UseWaitCursor correctly
This thread is really too long... The problem was that thread hangs, and now assertion failure. I am completely lost. Just make small threading exercise without BackgroundWorker, better in C#, and you will understand everything.
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Alex F
This thread is really too long... The problem was that thread hangs, and now assertion failure. I am completely lost.
Yes, well, the two are related, however: The assertion failure is caused by a wait on an EventWaitHandle that is supposed to be set by the BackgroundWorker (more particular: its RunWorkerCompleted() event handler) timing out.
Sorry for confusing you... :o
-
Re: How to UseWaitCursor correctly
The main problem was you were blocking the main UI thread by waiting on an event. I removed the DoEvent( ) hacks because they aren't necessary.
I also restructured the code to use a modeless progress form rather than the modal one.
I removed the Diagnostic timer methods. If you want to put them back in, put the start in the DoCount and the stop in the BackgroundCompleted handler.
Here's the updated code:
Code:
#include "stdafx.h"
#include "Form1.h"
using namespace MCppTest;
System::Void Form1::bnAbout_Click(System::Object^ sender, System::EventArgs^ e)
{
MessageBox::Show("Eri's erstes .NET-Programm! <puh>");
}
System::Void Form1::bnCount_Click(System::Object^ sender, System::EventArgs^ e)
{
if (openFileDialog1->ShowDialog(this)==System::Windows::Forms::DialogResult::OK)
{
DoCount((FileStream ^)openFileDialog1->OpenFile());
}
}
System::Void Form1::bnClose_Click(System::Object^ sender, System::EventArgs^ e)
{
Close();
}
System::Void Form1::DoCountWorker(FileStream ^fsInfile,BackgroundWorker ^bw,DoWorkEventArgs ^e)
{
BinaryReader brInfile(fsInfile);
const int nBufferSize=512;
array<Byte> ^abBuffer;
int nPercent = 0, nLastPercent = -1;
do
{
abBuffer = brInfile.ReadBytes( nBufferSize );
for ( int i = 0; i < abBuffer->Length; ++i )
++aulCounts[abBuffer[i]];
if ( ( nPercent = ( 100 * fsInfile->Position ) / fsInfile->Length ) != nLastPercent )
{
bw->ReportProgress( nPercent );
nLastPercent=nPercent;
System::Threading::Thread::Sleep( 250 ); // Slow the reading down so we can test it (remove for production)
}
}
while ( !bw->CancellationPending && abBuffer->Length == nBufferSize );
brInfile.Close();
fsInfile->Close( );
if (bw->CancellationPending)
e->Cancel = true;
}
System::Void Form1::DoCount(FileStream ^fsInfile)
{
// Disable the buttons
bnAbout->Enabled = false;
bnCount->Enabled = false;
bnClose->Enabled = false;
// Save off the file name
_fileName = fsInfile->Name;
// Reset the chart text
chart1->Titles[0]->Text = L"Byte frequency histogram";
// Zero out the chart data
Series ^serTarget = chart1->Series["Byte frequencies"];
serTarget->Points->Clear();
// Zero out the array
for ( int i=0; i < 256; ++i )
aulCounts[i]=0;
// Save off the current Cursor
_csrSaved = Cursor;
// Set the wait cursor on the main form (not the progress form)
Cursor = Cursors::WaitCursor;
// To enable calling back to the BackgroundWorker object
progress->bwParentWorker=bwDoCount;
progress->Show( this );
bwDoCount->RunWorkerAsync(fsInfile);
}
System::Void Form1::DoCount(String ^strInfile) {
DoCount(gcnew FileStream(strInfile,FileMode::Open,FileAccess::Read));
}
System::Void Form1::Form1_DragEnter(System::Object^ sender, System::Windows::Forms::DragEventArgs^ e)
{
if(e->Data->GetDataPresent(DataFormats::FileDrop))
e->Effect = DragDropEffects::All;
else
e->Effect = DragDropEffects::None;
}
System::Void Form1::Form1_DragDrop(System::Object^ sender, System::Windows::Forms::DragEventArgs^ e)
{
// Always use only the first file dropped and pass it to counting function:
DoCount(((array<String ^> ^) e->Data->GetData(DataFormats::FileDrop, false))[0]);
}
System::Void Form1::bwDoCount_DoWork(System::Object^ sender, System::ComponentModel::DoWorkEventArgs^ e)
{
DoCountWorker(safe_cast<FileStream ^>(e->Argument),dynamic_cast<BackgroundWorker ^>(sender),e);
}
System::Void Form1::bwDoCount_ProgressChanged(System::Object^ sender, System::ComponentModel::ProgressChangedEventArgs^ e)
{
progress->Text=String::Format("Processing file... {0}%", e->ProgressPercentage );
progress->progressBar1->Value = e->ProgressPercentage;
}
System::Void Form1::bwDoCount_RunWorkerCompleted(System::Object^ sender, System::ComponentModel::RunWorkerCompletedEventArgs^ e)
{
if (e->Error)
{
MessageBox::Show( this
, e->Error->Message
, "Error during file processing"
, MessageBoxButtons::OK
, MessageBoxIcon::Error );
}
// Progress dialog is still open: close it
if (progress->Visible)
progress->Hide();
if (!e->Cancelled)
{
chart1->Titles[0]->Text=String::Concat("Byte frequency histogram of ", _fileName);
Series ^serTarget=chart1->Series["Byte frequencies"];
for ( int i=0; i < 256; ++i )
{
serTarget->Points->AddXY( i, aulCounts[i] );
}
}
// Restore the cursor
Cursor = _csrSaved;
// Restore the buttons
bnAbout->Enabled = true;
bnCount->Enabled = true;
bnClose->Enabled = true;
}
Lastly, I'm a firm believer in the amount of whitespace used in formatting is directly proportional to the developer's ability to track down issues.
As such, I reformatted the code to add some whitespace.
-
Re: How to UseWaitCursor correctly
Thanks for correcting my code, Arjay. Great work, really invaluable! :cool: And your implementation is so pretty straightforward. I couldn't imagine that it could be done that simple. :)
After adding the two new members you introduced to the Form1 class and re-adding the instantiation of the Progress object in DoCount(FileStream ^) that got lost somehow :ehh: it worked instantly. I then could remove three or so members from the Form1 class that were unused now.
Did you add the Sleep() call in the worker function because you didn't have a 731 MB file to test the app with (or have a system where even processing that doesn't take a notable amount of time)? I found out that I could do some debugging pretty well without it.
I think one of the reasons why I got stuck with my screwed-up code was that I was stubbornly clinging too much to the concept of the modal progress dialog and trying to keep most work out of the RunWorkerCompleted() handler.
I must admit my "whitespace allergy" is most likely one of these "bad habits are hard to break" kind of problems. I started programming on a system that had a text display of 64 x 16 characters (and a "graphics" resolution of 128 x 48 brick-size "pixels", bi-level). (Not counting my previous experiments with programmable calculators.) Looks like I somehow still are stuck with this way of viewing things... :o
Now that the program is running fine, I've got only one question remaining before finally marking the thread resolved: As I (think I've) understood it, all windows of one GUI thread share a single common message pump. So, if my progress dialog could process messages, why then couldn't the main form? :ehh:
Ah, and... Of course you and Alex F are now credited in that app's About box. :)
-
Re: How to UseWaitCursor correctly
Quote:
Originally Posted by
Eri523
After adding the two new members you introduced to the Form1 class and re-adding the instantiation of the Progress object in DoCount(FileStream ^) that got lost somehow :ehh: it worked instantly. I then could remove three or so members from the Form1 class that were unused now.
I just renamed them by prepending an '_' to the existing variables. That way I could see identify them as a class field rather than a method param or local variable.
Quote:
Originally Posted by
Eri523
Did you add the Sleep() call in the worker function because you didn't have a 731 MB file to test the app with (or have a system where even processing that doesn't take a notable amount of time)? I found out that I could do some debugging pretty well without it.
I think I commented it, but you definitely want to remove the Sleep. I added that because the file I was working with didn't give me enough time to test the cancel and cursor functionality.
Quote:
Originally Posted by
Eri523
I think one of the reasons why I got stuck with my screwed-up code was that I was stubbornly clinging too much to the concept of the modal progress dialog and trying to keep most work out of the RunWorkerCompleted() handler.
Essentially with threading you need to do any UI work before the thread starts (like disable buttons) and do any 'post' thread operations (like reenabling buttons) in the backgroundCompleted handler.[
Quote:
Originally Posted by
Eri523
I must admit my "whitespace allergy" is most likely one of these "bad habits are hard to break" kind of problems. I started programming on a system that had a text display of 64 x 16 characters (and a "graphics" resolution of 128 x 48 brick-size "pixels", bi-level). (Not counting my previous experiments with programmable calculators.) Looks like I somehow still are stuck with this way of viewing things... :o
I really find code without whitespace very hard to read. I think that's why I missed that you were waiting on the event in the UI thread the first time around.
Quote:
Originally Posted by
Eri523
Now that the program is running fine, I've got only one question remaining before finally marking the thread resolved: As I (think I've) understood it, all windows of one GUI thread share a single common message pump. So, if my progress dialog could process messages, why then couldn't the main form? :ehh:
The reason was (and probably why the Cursor didn't work as well) because you were waiting on the thread completed [manual or auto-reset] event in the main UI thread. When you wait on an event with WaitOne, it blocks the main thread and the message pump doesn't run. You kind of overcame this partially by putting in a few DoEvent calls. I think Alex and I told you that DoEvent wasn't required (and having it in there indicated a problem).
I tell you coding in Managed C++ is a bit of a pain compared to coding in C#. The syntax for variables and namespaces is goofy. Something like:
Code:
using System::Threading;
private: System::Threading::AutoResetEvent^ _completedEvent;
In C#, the equivalent is:
Code:
using System.Threading;
private AutoResetEvent _completedEvent;
C# looks cleaner because there isn't all that namespace tomfoolery.
I was surprised to find out that intellisense doesn't work in managed C++. When you understand what needs to be done, but don't know the exact syntax (like me being unfamiliar with Managed C++), intellisense is sure handy.
Lastly, managed C++ organizes the classes into the std .h/.cpp file format, but I have to tell you, I find the project is really cluttered with both these files. In C#, there is only one file (.cs) for both the declaration and implementation - it's like half as many files to keep track of.
As a long time C++ developer that works a lot in C#, I tend to notice these sorts of things when switching between C++ and C#.
