|
Your while() loop is not relinquishing control of the CPU. Inside of that loop, you're going to need to poll for messages.
"One man's wage rise is another man's price increase." - Harold Wilson
"Fireproof doesn't mean the fire will never come. It means when the fire comes that you will be able to withstand it." - Michael Simmons
"Show me a community that obeys the Ten Commandments and I'll show you a less crowded prison system." - Anonymous
|
|
|
|
|
Hi,
1. first a general note: empty loops that spin the CPU until some condition is met are a bad idea as they waste CPU cycles. You should look first for an event-based approach, where the system tells you something happened rather than you permanently polling for it. If no event-based solution exists, your loop should relinquish the CPU by including a "sleep" of a couple of milliseconds at least, so other things can happen while you wait.
2. I don't know on which thread your while loop is executing; if it is on the main thread (the same one that would execute your OnTimer code), then there is no way the app can do both things at the same time: the OnTimer event will only get handled when the while has finished, causing a deadlock. Now read again my #1. If OTOH the while loop is running on another thread, then I don't know why your setup is failing.
3. If all you need is waiting for some input with a time-out, and still assuming there isn't an easy event-based approach, then I'd recommend this (pseudo-code!):
Time limit = now() + maximumWaitTime;
for(;;) {
try getting the input
if (success) break;
sleep(someDelay);
if (now() > limit) break;
}
|
|
|
|
|
Ahh, you have them in the same thread. Your while () loop is taking up all the thread time and the WM_TIMER messange cant get through.
You can multi thread this, or do a loop that checks for received data, then does a sleep(200), then checking again, for a max of 5 attempts.
Or an event, do a waitforsingleobject() then in a the code that sets m_rxbufferrcvd signals the event. (If you can get to that code).
==============================
Nothing to say.
|
|
|
|
|
If you have a long running loop, chewing up cycles waiting for something to happen, to keep the GUI responsive (and to let things like timer events happen), you should periodically let Windows process messages. Stick this somewhere in your 'while' loop. At the top or bottom are the usual places.
while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE))
{
TranslateMessage(&msg);
DispatchMessage(&msg);
}
And it is true that keeping the GUI and application functions separate in a thread is a good idea, sometimes it's just too much work to re-do an application's logic to introduce threading when none existed before. In those cases, this should do the trick and not be too "architecturally impure".
|
|
|
|
|
No, dont do this, you are reinventing the wheel and possibly introducing instability.
The message pump, which is what this is, is invisible in MFC apps (which I assume this guy is writing). Ansd in a C++ app having messages removed form the apps message queue and translated is at best ugly, and at worst dangerous, unless you KNOW that access to the apps message queue is multithread safe.
The quick solution here is a sleep() call inside the loop. It is sidely used, and isnt going to cause any problems.
==============================
Nothing to say.
|
|
|
|
|
Nonsense. Microsoft even documents this for dealing with exactly the kind of application.
Occasionally, an application needs to examine the contents of a thread's message queue from outside the thread's message loop. For example, if an application's window procedure performs a lengthy drawing operation, you may want the user to be able to interrupt the operation. Unless your application periodically examines the message queue during the operation for mouse and keyboard messages, it will not respond to user input until after the operation has completed.
Plus the user's loop is already preventing message processing, adding a Sleep() call will only *lengthen* the amount of time he's chewing up (real time, not cpu cycles obviously).
This is well documented and long standing solution to *some* (not all) unresponsive GUI / Timer queue issues. Even threaded applications can do this, only not from multiple threads at the same time, obviously.
|
|
|
|
|
Chuck O'Toole wrote: Plus the user's loop is already preventing message processing, adding a Sleep()
call will only *lengthen* the amount of time he's chewing up
No, quite the opposite. A sleep() surrenders the rest of that threads timeslice, it doesnt chew up cycles, it frees them. This function causes a thread to relinquish the remainder of its time slice [^]
And if Microsoft recomends that then I am surprised. I can just imagine the code. DispatchMessage() is locked, because WM_DRAW hasnt come back from your message handler, so in that handler you call another DispatchMessage(). Perhaps you have some other handlers that are slow, so you do the same there. What a mess.
Work intensive code should go in a seperate thread to the UI. Period, properly synchronised with the UI so the user has control.
Chuck O'Toole wrote: not from multiple threads at the same time, obviously.
And why, because the PeekMessage()/TranslateMessage()/DispatchMessage() is not thread safe?
Like I said, you risk instability.
==============================
Nothing to say.
|
|
|
|
|
Erudite__Eric wrote:
Chuck O'Toole wrote: Plus the user's loop is already
preventing message processing, adding a Sleep() call will only *lengthen*
the amount of time he's chewing up
No, quite the
opposite. A sleep() surrenders the rest of that threads timeslice, it doesnt
chew up cycles, it frees them. And if Microsoft recomends that then
I am surprised. Work intensive code should go in a seperate thread to the UI.
If you're going to quote me, use the whole quote, not the part that makes it sound like the opposite of what I said. The actual quote should be:
Chuck O'Toole wrote: Plus the user's loop is already preventing message processing, adding a Sleep()
call will only *lengthen* the amount of time he's chewing up (real time, not cpu
cycles obviously).
The parenthetical states that it's not the CPU Cycles but the "Clock On The Wall Cycles". Sleep() adds code to the loop, is a system call that adds more code being executed in the loop, and, even with Sleep(0) to quit the timeslice, still causes a complete scheduler cycle to look for other runnable threads before returning to this thread.
|
|
|
|
|
Chuck O'Toole wrote: Sleep() adds code to the loop,
Where as Peekmessage()/TranslateMessage()/DispatchMessage() does what?
Chuck O'Toole wrote: even with Sleep(0) to quit the timeslice, still causes a complete scheduler
cycle to look for other runnable threads before returning to this thread.
Happens all the time. It is what windows does. No problem there.
What Sleep() does allow is for the message handler to return and the message pump to do its job while, in this case, his code is waiting for data to be available.
==============================
Nothing to say.
|
|
|
|
|
1st, stop being a smarta$$, do I need a lawyer to review things for you to read? Clearly Sleep() adds code for *no benefit to this application* whereas *Peek/translate/dispatch* are *beneficial* to this app.
2nd, it's clear that you haven't read all the messages here. This guy isn't using threads, although lots of people are encouraging him to do so at the cost of redoing his application. That may or not be a good thing for *HIM*. I'm not debating that. He's not getting back to his message pump *because* he's doing processing in his GUI handler. Sleep ain't gonna help that because there's no Message Pump to run. Period. He has to provide his own (or redesign his app). I showed him how to provide his own JUST AS MICROSOFT DOCUMENTS.
|
|
|
|
|
Chuck O'Toole wrote: Clearly Sleep() adds code for *no benefit to this application* whereas
*Peek/translate/dispatch* are *beneficial* to this app.
Sorry, thats total guff. Sleep() will do the job perfectly well with less lines of code and in a more logical fashion.
Chuck O'Toole wrote: He's not getting back to his message pump *because* he's doing processing in his
GUI handler. Sleep ain't gonna help that because there's no Message Pump to run
You are wrong. Putting a Sleep() in the dispatcher will let the message pump run because it will release the existing call to DispatchMessage().
Chuck O'Toole wrote: there's no Message Pump to run. Period. He has to provide his own
Of course there is! He wouldnt HAVE a Windows applicaiton without a message pump!
==============================
Nothing to say.
|
|
|
|
|
[updated to reflect a third experiment's results]
Erudite__Eric wrote: Of course there is! He wouldnt HAVE a Windows applicaiton without a message
pump!
I'm not that dumb. Maybe I should have said "no message pump for Sleep() to return to". Your statement that Sleep() will release the call to DispatchMessage() needs to be proven beyond simply asserting it is true so I ran an experiment.
I have a "regressison tester" application that does many long running things. In one case, similar to this one, I'm just cross checking some datafiles and I need to keep a "status / log window up to date with the progress". So I have this function called at times in the processing loop.
void CRegressionTesterDlg::ProcessWindowMessages()
{
MSG msg;
while (::PeekMessage(&msg, NULL, 0, 0, PM_REMOVE))
{
TranslateMessage(&msg);
DispatchMessage(&msg);
}
}
And I ran three tests, one with the code as originally written, once with Sleep() replacing the message loop, and once with all the code commentd out. All kept the window up to date when it was visible. However, the Sleep() case and the "no code" case both failed to process other windows messages, like minimizing, clicking on the task bar to bring the window back to the front, etc. All these are classic symptoms of not having the "message pump" run.
So, I think that Sleep() does not do as you assert, it has no effect on the Windows Message Processing. Furthermore, Microsoft's documentation on Sleep() explicitly warns against using it during functions called by message processing as it may cause a hang or delay in processing.
Personally, since Microsoft says to use PeekMessage() in their documentation for exactly these types of long running things, I'm sticking with that. And I think you need to run some experiments on your own.
modified 1-Nov-11 11:23am.
|
|
|
|
|
So you tested this by putting a sleep() in the one function that handles all the windows messages?
ANd you call this a valid test?
==============================
Nothing to say.
|
|
|
|
|
Whatever gave you that impression?
Have you been following all of this?
Of course that's not the place that handles all the windows messages NORMALLY. Just like any GUI based application, it's event driven by messages dispatched by the standard windows message pump. It's reacting to the "OnButtonClick()" events, just like anybody else.
One of the clicks causes a long running validation process. It's that process, already called by the DispatchMessage(), that needs to keep the GUI responsive while it munges on the data files and reports progress. (As a side note, it also cares about the cancel button which sets a cancelled flag).
The whole point of this thread of this post has been exactly that, a process, itself a "dispatched" message handler, that is taking a long time to do something. So a SECOND, NON-BLOCKING message pump is needed to allow for those backed up messages to be processed, JUST LIKE MICROSOFT SAYS TO DO.
So, yes it's a valid test. It's not the loop that processes all the messages, and if you've been reading for comprehension, you'd see that.
I no longer wish to play this game with you.
|
|
|
|
|
|
And don't forget this, which is what I've been saying
Using Messages and Message Queues[^]
Occasionally, an application needs to examine the contents of a thread's message queue from outside the thread's message loop. For example, if an application's window procedure performs a lengthy drawing operation, you may want the user to be able to interrupt the operation. Unless your application periodically examines the message queue during the operation for mouse and keyboard messages, it will not respond to user input until after the operation has completed. The reason for this is that the DispatchMessage function in the thread's message loop does not return until the window procedure finishes processing a message.
You can use the PeekMessage function to examine a message queue during a lengthy operation. PeekMessage is similar to the GetMessage function; both check a message queue for a message that matches the filter criteria and then copy the message to an MSG structure. The main difference between the two functions is that GetMessage does not return until a message matching the filter criteria is placed in the queue, whereas PeekMessage returns immediately regardless of whether a message is in the queue.
The following example shows how to use PeekMessage to examine a message queue for mouse clicks and keyboard input during a lengthy operation.
|
|
|
|
|
Oh, by the way, yes, what you suggest works, I have seen it myself, though implementing multiple message pumps in the code as soon as things get stickky is ugly.
Propper threading and coordination is the answer, as always.
I always laughed at Winzip. Once started, if you click anywhere in the window it brings up a 'do you want to abort'. It has done that for as lonkg as I have known it. They probably hook the mouse to do this, but it always made me laugh since it is so easy to do properly.
==============================
Nothing to say.
|
|
|
|
|
One last thing, FYI. That methodology existed as far back as Windows 3.1, long before threading existed in Windows. It was once the only way to do things. It is still applicable for non-threaded GUI applications.
Yes, when threads came along with Windows NT, that would have been a better way to approach things but it's not a absolute requirement nor the solution for every problem.
Anybody who implementes multiple message pumps, presumably in separate threads, is asking for trouble. However, there is an old saying, "Here's the rope....."
|
|
|
|
|
Thats the reason I disliked your suggestion, not that it doesnt work, but that it is a hack, a hack sugested by Microsoft. A hack based on a very old model, and one I dislike.
Sorry for any umbridge caused, I didnt mean it. I just like to see things done properly.
==============================
Nothing to say.
|
|
|
|
|
Chuck O'Toole wrote: Whatever gave you that impression?
If you name a function ProcessWindowsMessages() dont be surprised if people assume it processes windows messages.
I dont have a problem with reading comprehension, I have a problem when people can't describe their code accurately.
==============================
Nothing to say.
modified 2-Nov-11 4:33am.
|
|
|
|
|
Chuck O'Toole wrote: So, I think that Sleep() does not do as you assert, it has no effect on the Windows Message Processing. Furthermore, Microsoft's documentation on Sleep() explicitly warns against using it during functions called by message processing as it may cause a hang or delay in processing.
I know that MSDN is sometimes wrong, but what I read about Sleep() , and what you may be already saying, is that it suspends the execution of the current thread, meaning that the current thread does NOTHING. So, in effect, it halts execution and processes no messages.
"One man's wage rise is another man's price increase." - Harold Wilson
"Fireproof doesn't mean the fire will never come. It means when the fire comes that you will be able to withstand it." - Michael Simmons
"Show me a community that obeys the Ten Commandments and I'll show you a less crowded prison system." - Anonymous
|
|
|
|
|
Nah, it's not wrong, it's exactly what Sleep() does. It has no knowledge of the context of what the application is doing (message dispatch or whatever else), it just stops running for the specified amount of time.
|
|
|
|
|
In addition to the other replies....
There's rarely a need to sit in a loop on the UI/message-loop thread. Waiting on I/O should be done on separate thread(s). IMO, learn how to use threads and thread synchronization objects like events and your app will be much better behaved (not eating CPU cycles doing nothing). Even a PeekMessage loop is going to peg a virtual processor unnecessarily. At least use GetMessage() if you can
Mark Salsbery
|
|
|
|
|
Yes, PeekMessage() is CPU intensive but GetMessage() blocks. PeekMessage() allows your GUI to be "responsive" incase the user clicks on something, GetMessage() waits UNTIL the user clicks on something. (Yes, I know "Clicks or types or something")
|
|
|
|
|
Chuck O'Toole wrote: PeekMessage() allows your GUI to be "responsive" incase the user clicks on something
All messages go through either function (unless filtered). If not there would be a ton of unresponsive apps that use getMessage() in their message loop(s)
Mark Salsbery
|
|
|
|
|