Solved

unhandled exception on a printf statement

Posted on 2000-03-29
18
705 Views
Last Modified: 2013-12-14
I am writing a complicated program with a server and multiple clients.  The server crashes on a line that looks like this:

printf("Done parsing input\n");
The call stack shows the error in _flsbuf where in assembly I found the ecx register to be NULL so I can see why it would crash, but how on earth did this happen, it seems to be a simple printf statement.

Another interesting thing about this, is that the clients communicate with the server and send data that needs to be parsed.  This is in an infinite loop, and it goes through successfully 20-30 times before it crashes.

Any ideas?

Curtis
0
Comment
Question by:curtisn
  • 6
  • 6
  • 2
  • +4
18 Comments
 
LVL 6

Expert Comment

by:graham_k
ID: 2667603
"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).
0
 
LVL 15

Expert Comment

by:Tommy Hui
ID: 2667605
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.
0
 

Author Comment

by:curtisn
ID: 2667763
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
0
 
LVL 5

Expert Comment

by:pitonyak
ID: 2667891

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?

*

0
 
LVL 6

Expert Comment

by:graham_k
ID: 2668238
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.
0
 
LVL 3

Expert Comment

by:MDarling
ID: 2668355
this looks like a multithreading synchronisation problem...
0
 
LVL 30

Expert Comment

by:Zoppo
ID: 2668851
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
0
 
LVL 22

Expert Comment

by:nietod
ID: 2669166
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.
0
 
LVL 22

Expert Comment

by:nietod
ID: 2669184
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?
0
Highfive + Dolby Voice = No More Audio Complaints!

Poor audio quality is one of the top reasons people don’t use video conferencing. Get the crispest, clearest audio powered by Dolby Voice in every meeting. Highfive and Dolby Voice deliver the best video conferencing and audio experience for every meeting and every room.

 

Author Comment

by:curtisn
ID: 2671497
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
0
 
LVL 22

Expert Comment

by:nietod
ID: 2671565
>> 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.
0
 
LVL 22

Expert Comment

by:nietod
ID: 2671573
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


0
 

Author Comment

by:curtisn
ID: 2672054
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??)
0
 
LVL 22

Accepted Solution

by:
nietod earned 400 total points
ID: 2672910
>> I am using CreateThread(),
That can definitly cause problems like this.  (however you may still have other problems too, but you have to fix this!)

In a C/C++ program (in VC or BC or BCB) you have to start a new thread using the _beginthread() or _beginthreadex() functions.  These functions will call CreateThread() to have windows create thread for you, but they do other important things.  In particular they initialize the RTL--Run time library--for the new thread.  The RTL contains a lot of static data that must be initialized on a thread-by-thread basis.  

Make that change and see if it improves things.  (even if it doesn't fix everything, keep the change!)
0
 

Author Comment

by:curtisn
ID: 2673705
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
0
 
LVL 22

Expert Comment

by:nietod
ID: 2673763
>> 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?)
0
 

Author Comment

by:curtisn
ID: 2674212
Adjusted points from 300 to 400
0
 

Author Comment

by:curtisn
ID: 2674213
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
0

Featured Post

Threat Intelligence Starter Resources

Integrating threat intelligence can be challenging, and not all companies are ready. These resources can help you build awareness and prepare for defense.

Join & Write a Comment

  Included as part of the C++ Standard Template Library (STL) is a collection of generic containers. Each of these containers serves a different purpose and has different pros and cons. It is often difficult to decide which container to use and …
Jaspersoft Studio is a plugin for Eclipse that lets you create reports from a datasource.  In this article, we'll go over creating a report from a default template and setting up a datasource that connects to your database.
The viewer will learn how to use NetBeans IDE 8.0 for Windows to connect to a MySQL database. Open Services Panel: Create a new connection using New Connection Wizard: Create a test database called eetutorial: Create a new test tabel called ee…
THe viewer will learn how to use NetBeans IDE 8.0 for Windows to perform CRUD operations on a MySql database.

747 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

11 Experts available now in Live!

Get 1:1 Help Now