Click here to Skip to main content
15,891,253 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
Server is use WSAAsyncSelect TCP mode .

Here is the part of code:

C++
BOOL CServerDlg::InitSocket()
{
	WSADATA	wsaData; 
	int		iErrorCode; 
	if(WSAStartup(MAKEWORD(2,2),&wsaData))
	{
		CString strError;
		strError.Format(_T("WSAStartup failed: %d"),GetLastError());
		AddSEHLog(strError);
		WSACleanup(); 
		return   FALSE; 
	} 

	m_socket=WSASocket(PF_INET,SOCK_STREAM,0,NULL,0,0);
	if(m_socket==INVALID_SOCKET) 
	{
		CString strError;
		strError.Format(_T("WSASocket failed: %d"),GetLastError());
		AddSEHLog(strError);
		WSACleanup(); 
		return FALSE; 
	}

	SOCKADDR_IN sockServerAddr;
	DWORD		dwPort = GetListenPort();

	sockServerAddr.sin_family =AF_INET; 
	sockServerAddr.sin_addr.s_addr=htonl(INADDR_ANY);
	sockServerAddr.sin_port= htons(dwPort); 
	
	if(bind(m_socket,(LPSOCKADDR)&sockServerAddr,sizeof(sockServerAddr)) == SOCKET_ERROR)
	{
		CString strError;
		strError.Format(_T("bind failed: %d"),GetLastError());
		AddSEHLog(strError);

		closesocket(m_socket); 
		WSACleanup(); 
		return FALSE; 
	}

	if(listen(m_socket,64) == SOCKET_ERROR)   
	{
		CString strError;
		strError.Format(_T("list failed: %d"),GetLastError());
		AddSEHLog(strError);
		return FALSE; 
	}

    // this is the custom message : WM_SERVER_ACCEPT
	iErrorCode = WSAAsyncSelect(m_socket,m_hWnd,WM_SERVER_ACCEPT,FD_ACCEPT); 
	if(iErrorCode == SOCKET_ERROR)   
	{
		CString strError;
		strError.Format(_T("WSAAsyncSelect failed: %d"),GetLastError());
		AddSEHLog(strError);
		return FALSE; 
	} 

	AddSEHLog(_T("server start success !"));
	return TRUE; 
}


C++
LRESULT CServerDlg::OnAccept(WPARAM wParam, LPARAM lParam)
{
    if(WSAGETSELECTERROR(wParam)) 
	{
		DeleteClient(wParam);
		ReFreshClient();
		return 0L; 
	} 
	
	if(WSAGETSELECTEVENT(lParam) == FD_ACCEPT)
	{
		SOCKADDR_IN	m_sockClientAddr; 
		int			nCount = 100;
		int			len=sizeof(m_sockClientAddr); 
		CString		strIP = _T("");
		CString		strPort = _T("");
		int			nLen;
		HANDLE		hThread = INVALID_HANDLE_VALUE;

        SOCKET		Client   =   accept(m_socket,(LPSOCKADDR)&m_sockClientAddr,&nCount); 
		RECVPARAM   recvParam;

		recvParam.m_sock = Client; 
		recvParam.m_hWnd = m_hWnd;

		m_cArrayList.Add(recvParam);

		hThread = CreateThread(NULL,0,RecvProc,(LPVOID)&recvParam,CREATE_SUSPENDED,NULL);
		SetThreadPriority(hThread,THREAD_PRIORITY_NORMAL);
		ResumeThread(hThread);
		WaitForSingleObject(hThread,INFINITE);

		CloseHandle(hThread); 
		ReFreshClient();
	} 
	
    return 0L; 
}

LRESULT CServerDlg::OnReadClose(WPARAM wParam, LPARAM lParam)
{
	if(WSAGETSELECTERROR(lParam))
	{
		DeleteClient(sockClient);
		ReFreshClient();
		return -1;
	}

	//////////////////////////////////////////////////////////////////////////
	switch (WSAGETSELECTEVENT(lParam)) 
	{ 
	case FD_READ: 
		{
			RECVPARAM *pRecvParam  = new RECVPARAM; 
			pRecvParam->m_sock     = sockClient; 
			pRecvParam->m_hWnd     = m_hWnd;
			pRecvParam->m_pMainDlg = this;
			
			HANDLE   hThread = CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)ReadThreadProc,pRecvParam,CREATE_SUSPENDED,NULL);
			SetThreadPriority(hThread,THREAD_PRIORITY_NORMAL);
			ResumeThread(hThread);
			Sleep(100);
			CloseHandle(hThread);
		}
		break; 

	case FD_CLOSE: 
		{
			DeleteClient(sockClient);
			ReFreshClient();
		} 
		break; 
	}

	return 0L; 
}


C++
//////////////////////////////////////////////////////////////////////////
// Here is to read the buffer from client
// Paser the packets.
DWORD CServerDlg::ReadThreadProc(LPVOID pParam)
{
	RECVPARAM	*pRecvParam = (RECVPARAM *)pParam;
	SOCKET		sock  = pRecvParam->m_sock; 
	HWND		hwnd  = pRecvParam->m_hWnd; 
	CServerDlg	*pDlg = (CServerDlg *)((RECVPARAM*)pParam)->m_pMainDlg; 

	if(MyWSARecvFrom(sock,&wsabufRecv,1,&dwRead,&dwFlag,(SOCKADDR*)&addrFrom,&len,NULL,NULL)== SOCKET_ERROR)
	{
		::PostMessage(hwnd,WM_RECVDATA,0,(LPARAM)wsabufRecv.buf); 
		delete[] wsabufRecv.buf;
		delete pRecvParam;
		return -1; 
	}

	::PostMessage(hwnd,WM_RECVDATA,(WPARAM)sock,(LPARAM)wsabufRecv.buf); 

	//////////////////////////////////////////////////////////////////////////
	// Paser the packets
	pszRecv = clientCMD.PaserData(sock,wsabufRecv.buf);
	delete pRecvParam;
	delete [] wsabufRecv.buf;
	wsabufRecv.buf = NULL;
	return 0;   
}



Here is a bug:

If many client(about 10/sec) connect the server when in the same time

the server's window cannot response anymore, and it will can't response the normal clients request.

I think this like WEB DDOS Attack. I worry about this bug for a long time.

Is this have to changed into IOCP mode ?

Is there any idea ?
Posted

1 solution

Your code is poorly designed. Why are you creating a new thread for each new connection? How many connections do you expect to be able to process if you consume that many resources? And why are you waiting with a time of infinity for the thread to end inside OnAccept()? If the socket is blocked... you will effectively freeze the entire application!

Everything is wrong... because it is a poor architecture. Highlight all of the code and press the delete key and redesign something new that does not create a new thread for each new connection... and does not wait for infinity inside your OnAccept() function.

Best Wishes,
-David Delaune
 
Share this answer
 
Comments
JackDingler 24-Feb-12 17:00pm    
Right. It would be better to have a thread pool service the connections.
Albert Holguin 28-Feb-12 1:00am    
Creating a new thread isn't the main problem, its waiting on the independent thread to finish that's the problem... if you have to wait for it to finish, what's the point of creating a thread. There are some cases where it would be desirable to thread out independent connection sessions, however op is definitely doing it all wrong.
[no name] 28-Feb-12 7:59am    
Creating a new thread on each new connection at a connection rate of 10/sec is not a problem? Do you really believe this? Each thread will consume 1MB memory for stack space and the rapid creation/deletion of threads will negatively influence context switching and page faults. It is not a very good design for a high-load server. As Jack suggested he should consider using a thread pool.

I would consider the WaitForSingleObject inside the OnAccept() a design flaw and this is what probably caused him to come ask for help. I am very happy to assist him and to also include other suggestions about architectural design changes.
Albert Holguin 28-Feb-12 11:38am    
Just like I said in my message earlier, its not the thread creation that's the main problem its the waiting. Creating a thread is always going to be slower than using one that's already been created, that's a fact, but it still doesn't change the fact that his main design flaw was in creating a worker thread then waiting for it to finish.
[no name] 28-Feb-12 20:46pm    
Thank you. I really appreciate your feedback.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900