Loop generates never-ending sockets and threads, need help debugging
Kurt
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 creates 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 client and server both share similar code, but I'll just post the server-side.
void CTransferServer::startCommandThread() {
int iCommandPort = _wtoi(configHandler->getTextValue(L"TransferCommandPort"));
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 command 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, transferCommandWorkerThread, params, 0, &dwGenericThread);
//HANDLE hWorkerThread = (HANDLE)_beginthreadex(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 passes it off to the thread. I am not 100% familiar with sockets and threads, but 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 communication between the server and client to get disconnected or in a deadlock state. 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?
--------------= Posted using GrabIt =----------------
------= Binary Usenet downloading made easy =---------
-= Get GrabIt for free from http://www.shemes.com/ =-
Gert-Jan de Vos
Your analysis is correct. This is not very nice C++ code but your
problem can be fixed by using _beginthread() instead of
CreateThread(). This takes care of closing the thread handle at the
end of the thread.
Branimir Maksimovic
> 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 creates 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 client and server both share similar code, but I'll just post the server-side.
>
> 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, transferCommandWorkerThread, params, 0, &dwGenericThread);
> //HANDLE hWorkerThread = (HANDLE)_beginthreadex(NULL, 0, transferCommandWorkerThread, params, 0, &iGenericThread);
> //WaitForSingleObject(hWorkerThread, INFINITE);
> //CloseHandle(hWorkerThread);
> }
> }
>
Hm, what happens if there are more than 1 pending connection requests?
This obviously doesn't work in case of Wait.... since than, no other
connection will be accepted till current client is processed...
Greets
Kurt
I believe that may be the case as when I have one client transferring
data to the server, another client gets an error. I replaced the
clients with _beginthread() and am waiting for this current transfer
to finish, though I guess this would be a good opportunity to test out
the resume functionality. While on the subject of threads and sockets,
do you have any books to recommend?
Thanks,
Kurt
Chris M. Thomasson
news:zYYan.3749$7S2...@news.usenetserver.com...
> 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
> creates 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 client and server both share similar code, but I'll just post the
> server-side.
[...]
Have you tried just `CloseHandle()' right after the call to
`_beginthreadex()'? This would be equivalent to creating the thread in a
detached state. You don't need to wait for a detached thread to complete. If
that is not compatible, then you can call `CloseHandle()' right before you
return from the thread entry function. Also, FWIW, you only need
`_beginthreadex()' if the thread uses the CRT...
[...]
Kurt
> "Kurt" <n...@given.com> wrote in message
I do not think the projects are using any CRT libraries, at least none
listed here: http://msdn.microsoft.com/en-us/library/abx4dbyh%28VS.80%29.aspx
I did not know that you could run CloseHandle() after creating the
thread, I assumed that it would cause it to stop. The reason I looked
into _beginthreadex() is because I read it was recommended over
CreateThread() due to some known memory leak. I have a few clients
running the beginthread() version, I'll see how it goes over the
weekend and if it doesn't close the threads properly, or causes
issues, I'll try CreateThread()/CloseHandle() in tandem.
Thanks.
Branimir Maksimovic
While on the subject of threads and sockets,
> do you have any books to recommend?
Unix network programming, Richard Stevens.
http://en.wikipedia.org/wiki/W._Richard_Stevens
Greets
Maxim Yegorushkin
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 handles.
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
Jorgen Grahn
Although that's for Unix, and the poster appeared to use some other
system. Stevens covers Unix and its variations, but not e.g. Windows.
For threads, I think you need more than Stevens -- I don't think he
teaches you to avoid all the pitfalls which makes all threaded code
I've seen a complicated, inefficient, and broken mess.
/Jorgen
--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .
Brian
> On Sat, 2010-02-06, Branimir Maksimovic wrote:
> > Kurt wrote:
> > While on the subject of threads and sockets,
> >> do you have any books to recommend?
>
> > Unix network programming, Richard Stevens.
> >http://en.wikipedia.org/wiki/W._Richard_Stevens
>
> Although that's for Unix, and the poster appeared to use some other
> system. Stevens covers Unix and its variations, but not e.g. Windows.
>
> For threads, I think you need more than Stevens -- I don't think he
> teaches you to avoid all the pitfalls which makes all threaded code
> I've seen a complicated, inefficient, and broken mess.
>
I'm no threading expert, but I've seen some multithreaded
applications that I don't think were inefficient or a
broken mess. I've also seen software that used more
threads than seemed necessary. It could have been called
messy, but I don't think it would have been appropriate
to call it a broken mess.
Brian Wood
http://webEbenezer.net
James Kanze
> On Sat, 2010-02-06, Branimir Maksimovic wrote:
> > Kurt wrote:
> >> While on the subject of threads and sockets, do you have
> >> any books to recommend?
>
> > Unix network programming, Richard Stevens.
> >http://en.wikipedia.org/wiki/W._Richard_Stevens
> Although that's for Unix, and the poster appeared to use some
> other system. Stevens covers Unix and its variations, but not
> e.g. Windows.
I've heard that sockets are similar enough under both that you
might be able to use some of the Stevens under Windows. I'm not
sure, however.
> For threads, I think you need more than Stevens -- I don't
> think he teaches you to avoid all the pitfalls which makes all
> threaded code I've seen a complicated, inefficient, and broken
> mess.
I'm not too familiar with this particular book of his, but in
general, Stevens delves into the details of each individual
request, and does not address architecture or larger scale
issues. The best book I've seen for threading (where program
organization is very, very critical) is _Programming with POSIX
Threads_, by David Butenhof. As the name indicates, the book is
purely Unix, but as far as I can tell, most of the issues it
explains are identical under Windows. Only the names of the
functions change.
--
James Kanze
Kurt
wrote:
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