unhandled exception on a printf statement

"This is in an infinite loop, and it goes through successfully 20-30 times before it crashes. "

You answered your own question. You are repeatedly going through the infinite loop until you run out of stack space; then "kaboom!".  There's nothing at all wrong with your printf statement. If you remove it, the crash will just happen on another statement, which is also O.K.

Try to find out why it's looping infinitely & correct that. If it _is_ necessary to loop many times, but not infinitely, try to find out how to tell your compiler to allocate more stack space.

If I am misunderstanding & you intend it to be infinite, then please post the code within the loop. Something in it is grwoing the stack, but not shrinking it again (could it be allocating, but not freeing memory? Sounds like the stack, though).

More than likely, you are corrupting the stack. With stack corruption, the bug doesn't manifest itself right away. You should check your functions to make sure you are not overwriting local variables.

Okay this is an idea I think might be happening, my next question is how do you check the stack or is there a function to determine how much stack space there is left.  The problem is this is on the server so we do want it to be infinite.  As in it isn't a loop per say, but each time someone connects we must handle the request and we don't want a limit on the number of requests.  

The code was written in Visual C++ 6.0
Here is the Client function
DWORD Client(ClientThreadData * data)
{
      char buffer[2048];
      char tempbuffer[2048];
      char *ptbuff;
      char *pbuff;
      bool cont;
   int  bufflen;
   int tbufflen;
   DWORD retval;
   SOCKET s = data->s;
      //*** Receive command messages
      retval = WaitForSingleObject(data->hMutex, INFINITE);
      //only used for multiple threads on the same team.
      do
      {
            
            bufflen = 0;
            memset(buffer, 0,2048);
            memset(tempbuffer, 0,2048);
            
            pbuff = strstr(buffer, "##");
            while (pbuff == NULL)
            {
                  bufflen += recv(s, &buffer[bufflen], 2048-bufflen, 0);
                  pbuff = strstr(buffer, "##");
            }
            //if more than one command read the next command then keep the rest in the buffer.
         if(bufflen < 0)
               return 0;

         ptbuff = pbuff;
         pbuff = buffer;
         //parse all the commands on the input buffer and send them to be taken care of.
         while (ptbuff != NULL)
         {
               //if multiple commands or not copy the first one into tempbuffer and add \r\n
                  strncpy(tempbuffer, pbuff, ptbuff - buffer-1);
//                  strcpy((&tempbuffer[ptbuff-pbuff-1]), "\r\n");
                  tbufflen = strlen(tempbuffer);
                  printf("%d Received command: %s\n", data->hThread, tempbuffer);

                  std::istrstream str( tempbuffer, tbufflen );
                  std::list<std::string> cmdlist;
                  std::string tok;

                  while( !str.eof() )
                  {
                        str >> tok;
                        cmdlist.push_back(tok);
                  }

                  //Print commands received
/*                  {
                        std::list<std::string>::iterator i;
                        for ( i = cmdlist.begin(); i!= cmdlist.end() ; ++i)
                              std::cout << (*i) << std::endl;
                  }
*/                  printf("Done parsing input\n");

                  //*** a quit command
                  cont = ParseBuffer( cmdlist,  data );
                  pbuff = pbuff + tbufflen+3;
                  ptbuff = strstr(pbuff, "##");
         }
            Sleep(10);
      } while( cont && !data->wc.getFinished() );
      ReleaseMutex(data->hMutex);
      closesocket( s );
   return 0;
}
In this function the other common places for it to crash are on the
cmdlist.push_back(tok) function inside the two while loops.

This is then passed onto the ParseBuffer function below.

bool ParseBuffer( std::list<std::string>& cmdlist, ClientThreadData * data )
{      
      while( !cmdlist.empty() )
      {
            std::string type = cmdlist.front();
            cmdlist.erase( cmdlist.begin() );
                  
            const char * c_type = type.c_str();
            //*** a move request to be buffered.
            if(strnicmp(c_type, "MOVE:", 5) ==0)
            {
                  if( type.length() > 5)
                  {
                        cmdlist.push_front( std::string(c_type + 5) );
                  }
                  data->ti.addMove(cmdlist);
            }
            //*** a move request to be buffered.
            else if(strnicmp(c_type, "ABS:", 4) ==0)
            {
                  if( type.length() > 4)
                  {
                        cmdlist.push_front( std::string(c_type + 4) );
                  }
                  data->ti.addAbsolute(cmdlist);
            }
            //*** a look command
            else if(strnicmp(c_type, "LOOK:", 5) ==0)
            {
                  if( type.length() > 5)
                  {
                        cmdlist.push_front( std::string(c_type + 5) );
                  }
                  data->ti.addLook(cmdlist);
            }      
            //*** a noop command
            else if(strnicmp(c_type, "NOOP:", 5) ==0)
            {
                  if( type.length() > 5)
                  {
                        cmdlist.push_front( std::string(c_type + 5) );
                  }
                  data->ti.addNOOP(cmdlist);
            }
            else if( strnicmp( c_type, "QUIT", 4) == 0)
            {

                  if(!data->wc.getFinished())
                  {
                        data->wc.setFinished();
                  }
               data->base->Decrement();
                  return false;
            }
            else      //if not a valid command just return
                  return true;
      };
      return true;
}

In here the most common place for it to crash is on the line:
cmdlist.erase( cmdlist.begin() );
at the begining of the while loop.

I am not very familiar with std::list and std::string and std::list<std::string> and all that fun stuff, so I imagine there could be problems in there.  Any help would be great, I am still looking at how to modify the stack size for Visual Studio

Thanks for the input and I hope for more

A few comments that may or may not matter here...

* Note that your two buffers will use 4K of stack space. The first thing that I would do to avoid the use of the stack is make them dyamic.

* I assume that there is no possibility that ClientThreadData* data could ever be null. You never check. How about data->hThread?

*

well, I agree with the previous comment, that it might be best to malloc() the buffers & take some sort of error recovery (deny the access) if it fails.

I doubt if just increasing the stack size will help, in this case.

Don't wory about std::list<>, etc. These are standard template library functions & should be well debugged. I'm not saying that I _never_ found a compiler bug, but I found a heck of a lot more in my own code.

this looks like a multithreading synchronisation problem...

To graham_k:
>You are repeatedly going through the infinite loop until you run out of stack space

How should this happen? Why should a loop (no matter if infinite or not) cause a stack overflow since there's no recursive call?

BTW, I don't think 4KB allocated on stack is that much nowadays, since Windows reserves 1MB virtual space per process...

ZOPPO

In

pbuff = strstr(buffer, "##");
                while (pbuff == NULL)
                {
                bufflen += recv(s, &buffer[bufflen], 2048-bufflen, 0);
                pbuff = strstr(buffer, "##");
                }

The code will not work correctly if buffLen goes past 2048.  You should at the very least include an assert() to test for this.


You seem to acknowledge this because you then have a

  if(bufflen < 0)
                   return 0;

but that is too late.  The damage will already be done.

This definitely is a problem. In

strncpy(tempbuffer, pbuff, ptbuff - buffer-1);
// strcpy((&tempbuffer[ptbuff-pbuff-1]), "\r\n");
tbufflen = strlen(tempbuffer);

you are strncpy()ing a source string whose total length is definiitley longer than the specified length.  That means that the string that formed in the destination will not be NUL terminated.  You will have to do this termination yourself.

In fact, your use of C-style strings is very questionable.  They are just terribly risk-prone.  Why not switch completely to STL strings?

Okay, I have made those modifications mentioned by nietod, bufflen in our case never actually got above 30 characters. and you are right I didn't make the string NULL terminated, so that is taken care of.
I also changed, the 4KB so it was allocated dynamically.

However we still have the problem.  

As we continue checking the errors thrown, it seems that when we get down to where the actual error is, somehow the address of the string is the value of the string. (or the first character thereof).  I know that sounds bizarre, but that is the case.  So we are trying to avoid the std::list structure and have instead decided to pass the stream (str) into ParseBuffer().  Other than that, does anyone have other implemntation ideas?

Thanks
Curtis

>> somehow the address of the string
>> is the value of the string.
What does that mean?

and what do you mean by address of "the string"?  is that the what the string pointer points to, or is that the address of the string pointer itself.

Are you using any global variables in the code?

If this is multi-threaded, how are you starting the thread?  You shoudl not use the windows thread functions for this (like CreateThread()), they don't initialize the RTL

the string issue was the address of the string pointer itself was the value of the string ? Weird?

I am using CreateThread(), what do you mean they don't initialize the RTL?  (real Time library??)

This might seem funny, but I can't get the _beginthreadex() function to compile

it looks like this
ctd->hThread = _beginthreadex(NULL, 0, ClientThread, (void*)ctd,0, &dword);
where ctd is:
ClientThreadData * ctd = new ClientThreadData(wc, *ti, s);
and dword is an unsigned int;

The error it gives is "cannot convert from 'unsigned long' to 'void *' " 
Any idea of how to make this work?  I have tried using _beginthread() but that won't work either, returns the same error.

Thanks,
Curtis

>> This might seem funny,
You must have a heck of a sense of humor.  :-)

The return value is a handle to the new thread, but it is not typed as a handle.  It is typed as a  unsigned long.  You priobably have "ctd->hThread" typed as a handle.  You will need to cast the return value to type HANDLE or change hThread.

(FYI, this is done so users can create threads without having to use windows.h.   Annoying?)

Adjusted points from 300 to 400

Thanks, that was the problem, I first changed CreateThread so it took more stack space, but I could tell the memory was being eaten up still.  Then when I switched to _beginthreadex and got all the correct libraries linked in, it worked!  I really appreciate your help thanks again.

Sincerely,
Curtis