Re: Loop generates never-ending sockets and threads, need help
debugging
On Feb 6, 1:45 pm, Maxim Yegorushkin <maxim.yegorush...@gmail.com>
wrote:
On 05/02/10 17:55, Kurt wrote:
Hi all,
I have a client/server project written by a hired contractor, but it's =
got a fairly big bug when it comes to dealing with thread and sockets. It c=
reates threads but never closes the handle to them, thus causing the server=
to accumulate hundreds of thousands of open handles in a day or so. The cl=
ient and server both share similar code, but I'll just post the server-side=
..
void CTransferServer::startCommandThread() {
int iCommandPort = _wtoi(configHandler->getTextValue(L"Transfe=
rCommandPort"));
SOCKET socCommand = CSocHandler::getServerSocket(iCommandPort)=
;
if (socCommand == INVALID_SOCKET) {
//cout<< "\nCould not create transfer command=
server socket."<< WSAGetLastError();
return;
}
if (listen(socCommand, SOMAXCONN)) {
//cout<< "\nCould not listen on transfer comm=
and port."<< WSAGetLastError();
return;
}
while (blStarted) {
SOCKET sa = accept(socCommand, 0, 0); =
// block for connection request
if (sa ==INVALID_SOCKET) {
break;
}
void **params = (void **) malloc(sizeof(void*)=
*2);
SOCKET *s = new SOCKET;
*s = sa;
params[0] = (void*)this;
params[1] = (void*)s;
DWORD dwGenericThread;
//unsigned int iGenericThread;
HANDLE hWorkerThread = CreateThread(NULL, 0, t=
ransferCommandWorkerThread, params, 0,&dwGenericThread);
//HANDLE hWorkerThread = (HANDLE)_beginthreade=
x(NULL, 0, transferCommandWorkerThread, params, 0,&iGenericThread);
//WaitForSingleObject(hWorkerThread, INFINITE);
//CloseHandle(hWorkerThread);
}
}
DWORD WINAPI transferCommandWorkerThread(LPVOID param) {
void **params = (void **)param;
CTransferServer *transferServer = (CTransferServer*)params[0];
SOCKET *commandSocket = (SOCKET*)params[1];
transferServer->handleCommandConnection(*commandSocket);
delete commandSocket;
free((void*)params);
return 0;
}
So, the while-loop creates a socket that accepts a connection and passe=
s it off to the thread. I am not 100% familiar with sockets and threads, bu=
t I have a general idea about them. As far as I can tell, the workerThread =
does close/delete the sockets properly, but the main loop never closes the =
handle to the thread. I tried adding WaitForSingleObject() and CloseHandle(=
) but it appears to close the thread prematurely. This causes the communica=
tion between the server and client to get disconnected or in a deadlock sta=
te. I heard that _beginthreadex() is better to use, but when I tried that, =
I believe it made matters worse.
Any ideas on how to debug this or perhaps a better way of writing this?
If you can you boost, you should. It can take care of all the
boilerplate code for starting a new thread and passing arguments to that
thread. Applying it to your problem:
#include <boost/bind.hpp>
#include <boost/thread.hpp>
struct CTransferServer
{
void startCommandThread();
void handleCommandConnection(SOCKET client);
bool blStarted;
};
void CTransferServer::startCommandThread() {
// ...
SOCKET socCommand;
while (blStarted) {
SOCKET sa = accept(socCommand, 0, 0);
if (sa ==INVALID_SOCKET)
break;
boost::thread( // start a new thread
boost::bind( // in a member function of this
&CTransferServer::handleCommandConnection,=
this
, sa // pass this extra argument
)
);
}
}
It is much shorter as the original version and does not leak thread handl=
es.
A gotcha, just in case, CTransferServer destructor will need to make
sure that no threads are running before it returns. Otherwise those
threads will end up accessing an already destroyed object.
--
Max
Hi Max,
Thanks for the brief introduction. I'll create a branch project and
implement boost into it and see how it goes. It's so hard for me to
debug this code since I didn't originally code it and the contractor
is essentially saying 'You made changes, you're SOL.' I came in today
and things seem to be going all right on the server. I need to check
the clients to see how they are running, though. I had a few bugs I
inserted into the code, which the contractor saw and I assume which is
why he's not willing to offer much help, that I removed.
I appreciate everyone's feedback and it seems I'm heading in the right
direction. It seems that people love Boost and I'm sure it's a good
resume builder.
Thanks,
Kurt