Program freezes when AfxGetApp()->PumpMessage( ) is called

Hi!

you should use

AfxGetThread()->PumpMessage().

instead of AfxGetApp().


In general, you should be very careful with calling MFC functions from other-than main thread. Although MFC has some classes supporting multithreading, many of its classes are designed with a single threaded application in mind.


Peter

I changed AfxGetApp to AfxGetThread and the program still freezes.  Thanks anyway.

I'm calling this function from a function that's not in the main thread, but the function itself is in the main thread.
So does that make the code when it executes in the main thread or the other-than main thread?
Its the main app class of the main thread that I want.

Hi!

Functions get executed in the thread that calls them. (Major Exception: Windows Message Handlers get executed by the thread that created the window)

I can't figure out any problem from the code snippet you gave.

Could you give some more code *AND* some details on what you do, what your threads (should) do, etc.?

So, if functions are executed in the thread that calls them then the Open function is called in a not-main thread and is therefore not in the main thread.
The AfxGetApp or AfxGetThread does work but it would return the not-main thread and therefore the PumpMessage function is
waiting for messages from the not-main thread, which there shouldn't be any, and it freezes.  Does this sound right?
But why does PeekMessage return true then?  It should be looking in the not-main thread as well shouldn't it?

My program's purpose is to connect to remote devices and allow to user to use like a command line interface to talk to devices.
The program creates a child window which has an instance of a thread which has an instance of a CEdit derived class inside it at start up.
This window takes in user commands and processes them by functions within the thread class (which would be not the main thread).
For every device connected to, another child window of the same design is created to display its output.  A CMDIChildWnd with a CWinThread derived
class inside it with a CEdit derived class in that.  The handle of the CEdit class is given to a device class, which is global and the devices outputs
are printed to its child window using the hwnd.
The Open code that I'm having a problem with is in a global class that is instantiated in a global device class, but the function is being called from a class that
is instantiated in the first thread class that initializes at startup.

I used to have some code in the OnCreate of the CEdit derived class that called SetFocus (I was trying to get the window to repaint).
I discovered that this made my program freeze when I opened up a child window when the application did not have focus.  I took out this code and then
the PumpMessage function in a different part of my program started to freeze each time I tried to open up a new device connection.

I've already asked some questions about my program if they help.  I've posted a ton of code there.
hWnd not attatched to CWnd??
Weird Crashing when application deactivates

I was kind of hoping for an easy answer to this one, but I may be in trouble again.  Thanks.

Here's a minor revision to the code you posted, but it probably won't change your situation (it's a bit cleaner):

#define CNUM_MaxWaitMs = 10000; // ten seconds

BOOL CMyClass::Open()
{
  DWORD nEndTick= GetTickCount() + CNUM_MaxWaitMs;

  // some code here to start the open proces (i presume...)

  while ( (nEndTick > GetTickCount()) && !m_target->GetConnected() ) {
    MSG msg;
    while ( ::PeekMessage( &msg,NULL, 0,0,PM_NOREMOVE) )  {
      if ( !AfxGetApp()->PumpMessage() ) {
        break;
      }
    }  
  }
}
--=-==--=-==--=-=-=-=-=
I think we have a problem that is similar to that in your other question https://www.experts-exchange.com/jsp/qShow.jsp?ta=mfc&qid=20003518

As I said there, you have made your program overly complex.  The view objects and the method of inputting commands should *not* have their own threads.  You need to decouple the device from the user interface or you will have continuing problems.

You need a "device object" and, *perhaps* each such object should run on its own thread.  The device object should do all of the talking to the device (which you never bother to describe -- that information could affect the quality of the help we provide).  This object would have methods such as "SendCommand" and "GetLastReply" and such so that the user interface objects can communicate with it, asynchronously, at any time.  It should have *no* direct connection to *any* User Interface object.  Instead, it can post messages or perhaps, make callbacks for certain time-critical events.

In particular, it is just dead wrong to have separate threads for each CEdit input control. The user can only be typing into one at a time, right?

-- Dan

I tried to post this once a few hours ago and it didn't work, so here it is again:

"You need a "device object" and, *perhaps* each such object should run on its own thread.  This object should do all of the talking to the device (which you never bother to describe -- that information
could affect the quality of the help we provide)."
I have one,  the device object uses a class to connect to the object and its the one of the two classes in the application I didn't write, which is probably why I didn't explain it, because I don't understand it completely.
Its built upon an IPWorks library that's included, I just call the functions like you mentioned, Send, etc. The connection class is passed an hwnd of a CEdit object and the output of the device is written to this window.    

I have a separate thread for the window that it displays its output to because I thought that was necessary to do if I wanted to be able to do something like type in commands or run a script while at the same
time output is coming from a device and needs to be displayed on the screen.

" In particular, it is just dead wrong to have separate threads for each CEdit input control. The user
 can only be typing into one at a time, right?"

In this version of the application, yes, that's right.  But future versions have to allow for command "spawning" so two scripts can be run at the same time.  Or a script can be run while a user enters in input.
For each script I'd have to have a command window to display possible error messages or print out the script as it runs if they choose that option.
How do I do that without a different thread for each one?
Right now I have all the major code that processes each command in this secondary thread.  I've come to discover from reading comments on here that that is a bad thing.  So I will probably move
all of this to a separate class in my main thread, but what about when two commands need to be processed at the same time?
 
Oh, and also, I'm trying to use your suggestion of GetTickCount in my program and I'm getting a compiler error I can't figure out
error C2059: syntax error : '='
on this line:
DWORD nEndTick = GetTickCount() + CNUM_MaxWaitMs;
I didn't think there was anything wrong with the equals sign so I looked around for another syntax error near it and couldn't find it.
I also commented out that line and the program compiled fine, (besides telling me nEndTick didn't exist).  I don't understand.

Thanks for the help again.

headache
:(

Sorry JenniferMH,
You have reaffirmed my belief that nobody (most especially me) should post code that hasn't been actually compiled.  The first line should be:

#define CNUM_MaxWaitMs 10000

Also, please forgive my tone in the previous post.  My criticism is well-intended.

I think that you, or *anybody* who is "fairly new to MFC," would be well-served to write a simple single-threaded application to work out the basics of operating with this device.  No development time is ever lost by writing the simple version first, then tacking-on complexity, trying different things until, eventually, it becomes time to start over with a ground-up rewrite, employing what you have learned from the first pass.  The result is a clean, robust, codebase for public release.  Functionality first, then bells, then whistles.

Again, you omit the description of the device you are actually using.  But a little bit of research shows that IPWorks is in the business of making controls that access the Internet.  That implies the use of Winsock and *that* has several implications for multi-threaded applications.  I can't describe how many unsolved questions I've looked at here at EE and elswehere that have to do with "Why does my multi-threaded app halt/crash/blow-up inexplicably/randomly/always when I startup/shutdown/close-a-window/type-characters while using Windows Sockets/CHttpXxxXxx/InternetXxxx."  It has to do with how Winsock uses window messages -- usually to a kludgy hidden window with its own message pump -- for pseudo-asynchronous operation.  Separate threads obviously can work (witness IE), but there are many conceptual hurdles that get in the way.

>>...a command window to display possible error messages or print out the script... How do I do that without a different thread for each one?

Almost like magic:  If you send a message to a window, it displays what was sent to it... even if you have 15 windows open.  So if you send a line of a script, it is displayed.  If you type a word, each character gets sent and it, too, will be displayed.  If you send an error message... well, you get the drift.  

In my experience, it is best to use a secondary thread only when a single thread would block the main message pump.  And when multi-threading is necessary, you need to be *very careful* with resources including global variables, database classes, serial port I/O, etc.

There is another simple gambit that simulates multi-threading without the side-effects.  Use a window timer.  For instance, rather than:

while (!m_object->IsConnected() ) {
  //wait and hope
}

just have an OnTimer handler that checks ever 10th of a second to see if !m_object->IsConnected() ever goes TRUE.  Just a thought.

-- Dan

I should have caught the define line myself, I was so busy looking at the nEndTick line that I didn't even notice.
That's where it said the error was.

Thanks for the advice.  I think that in the future version I will take out both the thread used for the input window and the one used for the device output.
I think I understand now how it could be done without threads.  With all the problems you've pointed out and that I've read on here, and the very frustrating bugs I keep coming up with
its probably worth rewriting.

I will still have to add in another thread eventually so two scripts can run at the same time but I won't attacth the thread to a window, I'll just send messages to the window
when I want something to print on it.

It will be a fair bit of work to rewrite it and to figure out exactly how to do it but hopefully if I can get it working this way it should be much more reliable.

But for now I'm afraid I need a quick fix for the problem as it is, because I'd like to have this version at least mostly working (and it was almost, I swear), before I take weeks to rewrite it.

So I'll try your suggestion about using windows timer and see if it helps.  

Thanks again.

Hi! Try use native Win32 API
  MSG msg;
   while ( ::PeekMessage( &msg, NULL, 0, 0, PM_REMOVE ) )
   {
    TranslateMessage(&msg);
    DispatchMessage(&msg);
   }

Hello

It still freezes about 90% of the time when I changed it to that.
It looks like it freezes on DispatchMessage.
I think that code is pretty much what is in PumpMessage anyway.
Thanks though.

I'm still working on trying windows timers.

Hi JenniferMH,

If my guess is correct, there is an external message pump that may be processing the messages and mucking up the works.  If that is so, you can use code like:

while ( (nEndTick > GetTickCount()) && !m_target->GetConnected() ) {
  ::Sleep( 100 ); //  
}  

without causing your UI to freeze.  Or if it does feeze, it will be only for CNUM_MaxWaitMs.

In your code marked as

  //some code

Do you call the device object, passing in an HWND?  If so, see if they allow passing in NULL.  With socket apps, doing that forces the object to create its own hidden window for its behind-the-scenes message pump.  It might make a difference.

-- Dan

I thought of another try.  Call PumpMessage only when the Peeked messages is for your window or one of its children:

while ( ::PeekMessage( &msg, m_hWnd, 0,0,PM_NOREMOVE) )  {
  if ( !AfxGetApp()->PumpMessage() ) {
...etc...

-- Dan

Hello

I think I tried to use sleep once and it froze completely, which doesn't make a lot of sense so I'll try it again.

I've done something to totally wreck my program at the moment but as soon as I figure out what I did and fix it I'lll try both your suggestions and let you know.

Also, yes, I do pass the device object an HWND, but if I pass NULL to it then the device output won't be written to the screen.
 I guess I could always set the internal variable back to the HWND after I call Peek and Pump Message.

For the second suggestion, because the code is in a secondary thread.  If I pass the HWND of the window in this thread wouldn't it just process messages for this window
and not the MainFrame, wouldn't this make the program freeze?

I'll try them anyway and let you know...

Thanks

>>wouldn't it just process messages for this window and not the MainFrame... make the program freeze?

I think you are right.  But there might be a secondary message pump involved, so who knows. It's something I'd try.

-- Dan

Hello again.

I manged to fix whatever I had wrecked and try your suggestions.  Sleep did make the program freeze for the 10 or 20 seconds
while it tried to connect to the device.  It did unfreeze after that but the connection is never made.

changing NULL to m_hWnd in PeekMessage also made the program freeze.  I tried putting the HWND in there for the secondary thread and also
the HWND of the MainFrame, both froze.  It freezes in PeekMessage.

If I put SetFocus in the oncreate function of my CEdit derived class for the device output, the programs works, except for the original bug that I was
trying to fix, which is that the program freezes when it opens a new device when it does not have focus.  This bug, although bad, is better than it
freezing all the time!  But I'll keep trying.

Thanks again.

>>the original bug that I was trying to fix, which is that the program freezes when it opens a new device when it does not have focus.

I don't see anything about this in your original question.  It is certainly relavant!

Some socket-handling utilities use a GetActiveWindow() call when they need a window -- with the assumption that the window will remain active during the lifetime of the call.  

Sometimes that call can be avoided by passing-in an HWND to a different window.  If that is not possible, you might need to somehow avoid losing focus or getting closed while inside the call to MakeSOmeSortOfConnectionThatJenniferWantsKeptSecret() or whatever it is that is in the
  //some code
part of your posted code.

Incidently, programs often stop responding briefly while making a socket connection.  Even the dialer for IE becomes the foreground window and stays there, while connecting.  You can't even click the [Start] button for a while.  So maybe the connection freeze-up is something that can be tolerated.

-- Dan

hmm...  I keep posting comments but they never show up.
can anyone else see them?

ok then I'm trying it in chunks...Hello

Sorry for the slow response, I was having trouble logging in and posting comments.  Yes, it is relavent, I had thought I'd mentioned that in my original question, but I hadn't sorry.

"Sometimes that call can be avoided by passing-in an HWND to a different window."

That one confused me sorry, a window different than which window?  The active one?  I am passing to the connection class an HWND belonging to the child window I create for it.

I will post the  MakeSOmeSortOfConnectionThatJenniferWantsKeptSecret()
code, I wasn't trying to be stingy, I just didn't think it was that revealing, but now I think I'm wrong.

This is called after a new child window has been created with a thread inside it and a CEdit derived class inside that.  The HWND is the HWND of the CEdit class where the device output prints.
void CCmdConfig::CreateDevice(HWND hWnd)
{
  //create a new device with the information in the file
  CIteDevice *pDev = new CIteDevice(m_DevType, m_DevName, m_DevIP, hWnd, m_hwndMainFrm);

  if (pDev->m_pHandle == NULL)
  {
    AfxMessageBox("m_pHandle null in config create device");
    return;
  }

  //call the open function for the device's handle
  if (!pDev->m_pHandle->Open())
  {
    //set event so TakeAction code is not longer locked
    g_pEventDevCreated->SetEvent();
     
    //create a string with the device info
    CString * pinfo = new CString("");
    CString info = m_DevType + " " + m_DevName + " " + m_DevIP;
    pinfo = &info;
   
    //set the error number
    m_pErr->SetErrorNum(ERR_DEVNOTOPEN);

    //set been closed to true so the Destroy Window function for the child window doesn't
    //call the close command
    *g_pbBeenClosed = TRUE;
    //send a message to the mainframe to close the device window
    ::SendMessage(m_hwndMainFrm, WM_CLOSE_DEV_WND, (WPARAM) pinfo, NULL);
    //reset beenClosed so close and config work correctly next time
    *g_pbBeenClosed = FALSE;
    //leave
    return;
  }

  //wait for the handle to open
  g_pEventDevOpened->Lock();

  //add the device pointer to the device list
  m_pDevList->AddDevice(pDev);

  //set event to unlock the TakeAction function
  g_pEventDevCreated->SetEvent();
}

The pDev->m_pHandle->Open function is the one that contains the code I posted before.
Also, for some reason, when I time the amount to wait for the device to get connected, when I use your code with the GetTickCount() + CNUM_MaxWaitMs it freezes as well, but it doesn't when I use the CTime::GetSeconds, etc way.  (This is all with SetFocus on the CEdit derived constructor present, without it they both freeze).  I think they're both doing the same thing, when I tried to trace it with using GetTickCount it got out of Open and Create Device fine but accessed invalid memory later after a message handler had been called.
I also have a testfile which is how I know where it freezes, besides tracing it with the debugger which I've been doing as well.  I also know its not freezing on any of the events I have because of the tracing.
BOOL CEmrcTelnet::Open()
{
  //the target was already open
  if (m_target)
  {
    g_pContainer->m_pErr->SetErrorNum(ERR_OPENALREADY);
    return FALSE;
  }

  //create a new telentex object, pass it the hwnd so it can write to it.
  m_target = new CTelnetEx(m_hWnd);

  ASSERT(m_target);

  if (!m_target->InitConnection(m_IP))
  {
    g_pEventDevOpened->SetEvent();
    return FALSE;
  }

  ptestfile->OpenFile();
  ptestfile->WriteLine("in open");
  ptestfile->CloseFile();
  CTime m_Time = CTime::GetCurrentTime();
  //convert minutes to seconds and add to seconds to get the current time in seconds
  int currtime = m_Time.GetSecond() + m_Time.GetMinute() * 60 + m_Time.GetHour() * 60 * 60;
  //get the time to stop the connection
  int donetime = currtime + 20;

     // timeout after 20 seconds
  while (currtime < donetime && !m_target->GetConnected())
  {

    //processes normal messages while sleeping
    MSG msg;
    while ( ::PeekMessage( &msg, NULL, 0, 0, PM_NOREMOVE ) )
    {
      if ( !AfxGetThread()->PumpMessage( ) )
      {
        break;
      }
    }

    m_Time = CTime::GetCurrentTime();
    currtime = m_Time.GetSecond() + m_Time.GetMinute() * 60 + m_Time.GetHour() * 60 * 60;
  }

  /*This makes it freeze when I uncomment it and comment out the while loop above*/
 /* DWORD nEndTick = GetTickCount() + CNUM_MaxWaitMs;
  while ( (nEndTick > GetTickCount()) && !m_target->GetConnected() )
  {
    MSG msg;
    while ( ::PeekMessage( &msg,NULL, 0,0,PM_REMOVE) )  
    {
      if ( !AfxGetApp()->PumpMessage() )
      {
        break;
      }
    }  
  }*/

ptestfile->OpenFile();
  ptestfile->WriteLine("after while");
  ptestfile->CloseFile();

  if (m_target->GetConnected())
  {
    g_pEventDevOpened->SetEvent();
    return TRUE;
  }
  else
  {
    g_pEventDevOpened->SetEvent();
    return FALSE;
  }
}

Here is my OnCreate for my CEdit derived class
int CCLI::OnCreate(LPCREATESTRUCT lpCreateStruct)
{
  if (!m_bReadOnly)
  {
    //Create the CLI object with a default message and prompt
    lpCreateStruct->lpszName = "Welcome to EScripting\r\n\r\nEScripting>";
  }

  /*If this line is commented out, the program freezes in the open function during PumpMessage, if it's left in, and the CTime method of calculating the time is used, the program works fine, connection is made, output is displayed.  Except for the case when the application does not have focus when the device is created, and its window pops up, then the application freezes*/
  SetFocus();

     if (CEdit::OnCreate(lpCreateStruct) == -1)
          return -1;    
 
     return 0;
}

Confused yet?  I know I am.  The CTelnetEx class, by the way, is derived from an IPWorks class.  I can see its code, but not it's parents code, I'm assuming that's part of the library I'm including.  
Here is the code for InitConnection (I didn't write it by the way):
int CTelnetEx::InitConnection(CString &host)
{
     // anti-crash
     if (host == _T(""))
          return -1;

  int len = 0;
     int ret_code = 0;

  ret_code = SetWinsockLoaded(TRUE);

     if (ret_code) goto done;

  //close old connections (if any)
  if(GetConnected())
     {
          SetConnected(FALSE);
     }

  //set the remote host
     len = host.GetLength();
  ret_code = SetRemoteHost(host.GetBuffer(len));
  if (ret_code) goto done;

  //connecting
  ret_code = SetConnected(1);

     // check if connection is ready
     
     // error checking
     done:
    if (ret_code) {
     return FALSE;
    }
 
  return TRUE;    
}
I can't see GetConnected so it must belong to CTelnetEx's parent.

Also, having a brief freeze when the program connects is acceptable, but the program was freezing for the
entire time it was waiting to connect (10 to 20 seconds).  Isn't that a little long?  And the connection was
never actualy made when ::Sleep was used, GetConnect was never sucessful.

I hope this helps.  
Thanks again.

it must have been to long.
That's never a good sign...

Ok, thanks again.
I should have caught the REMOVE/NOREMOVE, I had changed it for someone elses suggestion and forgot to change it back.

I'm not sure about the session ID, I have had as many as 8 sessions open at once with successful results, and haven't had to assign a new ID.

You are right about the difficulty of debugging this program, I'm having problems and I can see it all, not to mention writing it.  I will, as I said before, rewrite it in the second version taking out all of the threads and see if I can get it working like that.  Then I'll add in threading to enable running two scripts at once.  Originally I didn't understand MFC threading very well (being the first application I've written) so I just put them in like that based on another example program.  Now I understand it a little better.
Anyway I think I'll put the program back to the way it was when I started having it freeze when it opened a device when it didn't have focus.  At least it mostly worked then.
Then I'll work on the newer less-threaded version when I'm done documenting.
My original problem wasn't really solved but you convinced me that its probably fairly impossible to solve. At least not without rewriting the program, which is actually a valid answer in itself, albeit a depressing one for me to hear.  So I'll give you the points.
Thanks for all your help again.

Thank you.  I wish you the best of luck!

-- Dan