CreateThread Causes Application Error

[FlyGuy1203]

I am creating a multi-threaded TCP/IP server that will be running on Windows Server 2003. The program accepts a connection and then spawns a new thread to handle each new incoming message.

The program can successfully recieve and respond to the first message, but the program unexpectedly closes after the second message is recieved. The event log gives me a standard windows application error with fault address 0x000030f6.

I'm using Windows threads, and I've narrowed it down so that I believe that the CreateThread call is causing the crash, however it fails on the second time this call is made, not the first.

It's notable that when the program runs on XP, using a test program as the client, with a constant stream of messages it runs fine. However, when I move it onto the Server box it generally has to wait for the second message, and it crashes after this message has been recieved.

any thoughts?

[firedraco]

Hmm, well some code would be nice...It *might* be that you are assigning the handle to some dataspace you don't have something...I can't really say much without the code though.

[FlyGuy1203]

#include <windows.h>
#include <winsock.h>
#include <stdio.h>
#include <iostream>
#include <string>
#include <cstring>
#include <sstream>
#include <fstream>
#include <vector>
#include <math.h>
#include <cstdlib>
#include <time.h>
#include "MBImessage.h"
#include "resource.h"

#define NETWORK_ERROR -1
#define NETWORK_OK 0

//Structure defenitions

struct request_struct
{
SOCKET socket_fd;
int nbytes;
unsigned char buf[256];
};
typedef struct request_struct request_t;

void producer(SOCKET socket_fd)
{
char fileName[20];
char date[9];
char time[9];
request_t *request;

_strdate(date);
_strtime(time);
time[2] = date[2] = time[5] = date[5] = '-';
int n = sprintf(fileName, "%s.%s.txt", date, time);
lFile = fopen(fileName, "w");

HANDLE newThread;

while (1)
{

request = get_request(socket_fd);
if (request != NULL)//if something has been read from the buffer
{
*******************THIS CALL CAUSES FAILURE*****************
newThread = CreateThread(NULL, 0, process_request_thr,(void *)request, 0, NULL);

}
else
{
fclose(lFile);
free(request);
break;
}
}
}

DWORD WINAPI process_request_thr(LPVOID arg)/* Just a wrapper function */
{
int j = process_request(arg);
return 0;
}


This is the section of code where the failure occurs. There is obviously a lot more but the rest seems to be functioning correctly. producer is called by the main controlling thread, and it spawns a new thread for each message recieved. get_request reads the messages off the socket defined by socket_fd. request is a struct that holds the socket descriptor the input buffer and the length of the input.

As of this post the program fails on the third call to CreateThread().

[firedraco]

It could be because you are assign a new value to newThread...which might leave data behind...it shouldn't cause a problem however. I would also check the arguments you are passing to it...they seem fine but I don't remember the exact syntax...maybe tomorrow I'll be more awake XP

[Mal Reynolds]

based solely on the code you posted i have a few ideas that may be giving you grief (also a few suggestions below):
I don't really see a problem with your call to CreateThread(), aside from possibly 'I)' below, but a compiler should catch that. Multithreaded apps are notorious for being difficult to debug, but hopefully an idea below may help you as I don't see an obvious problem with your CreateThread() call.

NOTE: i would just like to say, i have no idea how experienced a programmer you are, so if anything below seems 'duh' or redundant, i do very much apologize.

1) as soon as get_request returns a NULL value, your loop will exit, and the function will subsequently exit, this may or may not be the behavior you want (i.e. - you may want to continue listening for messages even though you don't have any to process in the mean time). Also, just make sure that get_request is allocating memory for your request_t struct, since 'request' is only a pointer to a structure, and must have memory allocated for it to actually store valid data, otherwise, you may be overwriting other memory.

2) which brings me to the next possibility: you are passing a pointer to the CreateThread() function that points to the 'request' structure. however, each pass through the while loop re-initializes that variable, thus depending on how long it takes each thread to process that information, or the thread scheduling on a certain machine or processor, and whether or not the thread copies that information to local storage, it may, or may not be using valid information. (see #3 just below also).

3) finally, related to #2 above: since I don't know the specific implementations of some of your functions... you may need thread synchronization. meaning, if two threads try to write to the file you have open, or change variables or memory, you may be getting problems there. above, i used the 'request' variable as an example. you need to lock out all but one thread as it accesses that memory, then release that memory, store new information in it, and allow the next thread to access it. even though it is just a pointer, if that pointer is changed before a thread/function is done with it, the result would be uncertain. a simple google search for "thread synchronization" should give you info if you need some, there are articles from MSDN, codeguru, and codeproject in the first several results.


here are a few other things that may help you:

I) probably just a typo, since a compiler should catch it... in your call to CreateThread(), the third parameter is a pointer to a thread function, not just the name of the function. You could use either &process_request_thr in the call, or even
LPTHREAD_START_ROUTINE lpThreadFunctionName = &process_request_thr; before, then pass
lpThreadFunctionName as the third parameter in the function call.

II) you should test 'newThread' to make sure that CreateThread succeeded and actually created a new thread. msdn doc on CreateThread: <http://msdn.microsoft.com/en-us/library/ms682453.aspx> see 'return value' section. You can even use GetLastError() to find out what happened if no new thread was created.

III) it may also be beneficial to check 'j' for some error to make sure process_request() did not fail.

IV) i see that you are trying to be good about memory, by doing
free(request); but if you look carefully at your conditional statements, i believe this can get executed even when request == NULL! this can be taken care of by just taking it out of the else and putting it in an
if( request != NULL ){free(request);} after.


ps - again, i'm sorry if any of this is obvious to you. take care, and good luck!