C / C++ / MFC

30-Jan-07 11:51

Alright, school is up back to the IOCP server Smile | :)

Everything works near to fine now, thanks to the good help i got from this site.
Anyways, there is one small problem/bug/design flaw left. I currently found a way around it but i dont like having ugly code with a way uglier solution.

Ok again im still using the IOCP with UDP.

Anyways, The problem is that i get trash in my completion ports. for example, i send something which goes fine by the way. After that i do a WSARecvFrom. Then i retrieve a IOCP notification pointing to the right extended overlapped. Now, the data inside is somethimes the right data i expected to recieve with the right size like 600 bytes, but somethimes it returns a size of 1 and no data unless i give it some time. So what do i do, i just continue and recieve a next notificication when recieving 1. this next notification contains the right data + size, so my first question, why did i get notified about my recv request before he comepletely recieved the data. a second problem, whenever i had the right data, it notifies me for like 2 extra times about it before nocking it off.

Anyone knows a NICE way to prevent this because its really ugly.

Thanks already!

Mark Salsbery30-Jan-07 12:21

30-Jan-07 12:21

Can you show the code you are using to look at the send completion packets - from the
GetQueuedCompletionStatus() call on?

If it truly returned indicating one byte received then you'd have to keep that one byte and
repost the recv for the remaining bytes. This doesn't make sense for UDP though, unless your
buffer sizes are messed up when you call WSARecvFrom().

Polity4h30-Jan-07 12:55

30-Jan-07 12:55

i stripped the functions in the main code so its easier to read. this is what happends. i got a list of virtual sessions, every virtual session can be used once at a time (logical) as soon as a virtual session is released, at will be reset to another host and a post the the IOCP will be made with the task set to Begin. I've checked and counted. it one virtual session, the enum will be set to Begin and a post will be made, then he calls the member function RetrieveServerInfo which sets the buffers associated with the virtual session only, set the enum to RequestSend which sends the data. this happends one time, then it goes to after send and then the request send. till now all those steps have been held 1 time per session. After that he goes to afterrecieve. this step he repeates for a few time, even if the session is released. it might be that i use the same session AND OVERLAPPED structure for the new server query.

				while (pThis->m_bKeepAlive){ <br />
					BOOL bIORet = GetQueuedCompletionStatus ( // Get a completed IO request<br />
						   pThis->m_hIOCompiltionPort,<br />
						   &dwIoSize,<br />
						   reinterpret_cast<LPDWORD>(pThis),<br />
						   &ptrOverlapped, INFINITE);<br />
					ptrOverlappedPlus = reinterpret_cast<_OVERLAPPEDVirtualSession*>(ptrOverlapped);<br />
					ptrSession = ptrOverlappedPlus->pClientContext;<br />
<br />
					if (!bIORet){<br />
						__REPORT("Winsock error code: %i\n", GetLastError());<br />
						Gameye::Library::Thread::CMPThread::Sleep(0);<br />
					}<br />
					else{<br />
						CAbstractServerClass* ptrGameClass = (*pThis->m_ptrGameManager)[ptrSession->getNDServer()->GameID];<br />
						if (ptrGameClass == NULL){<br />
							__REPORT("Game is unknown, probably removed");<br />
						}<br />
						else{<br />
							objToolset.Session = ptrSession;<br />
							switch(ptrSession->getTask()){<br />
								case Begin: //Starts the procedure<br />
									pThis->m_objAddGameToolset.Session = ptrSession;<br />
									ptrGameClass->RequestServerInfo(ptrSession->getNDServer(), &pThis->m_objAddGameToolset);<br />
									break;<br />
								case RequestSend: //Request to send a packet<br />
									dwIoSize = 0;<br />
									iTemp = 0;<br />
									ptrSession->setStatus(AfterSend); <br />
									if (WSASendTo(ptrSession->getSocket(), <br />
										ptrSession->getWSABuffer(),<br />
										1,<br />
										&dwIoSize,<br />
										0,reinterpret_cast<sockaddr*>(ptrSession->getAddress()),<br />
										sizeof(sockaddr_in),<br />
										ptrSession->getOverlapped(), NULL) == -1)<br />
									{<br />
										int iErrorCode = WSAGetLastError();<br />
										if (iErrorCode == ERROR_IO_PENDING){<br />
											break;<br />
										}<br />
										else{<br />
											__REPORT("Recieving failed because: %i", WSAGetLastError());<br />
											pThis->ReleaseSession(ptrSession);<br />
										}<br />
									}<br />
								case AfterSend: //Call sending event<br />
									if (dwIoSize <= 1)<br />
										break;<br />
<br />
									ptrSession->incSendBytes(dwIoSize);<br />
									if (ptrGameClass->RetrieveServerInfo(ptrSession->getNDServer(), &objToolset) < 0){<br />
										__REPORT("%s->RetrieveServerInfo() failed", ptrGameClass->Gamename());<br />
										pThis->ReleaseSession(ptrSession);<br />
									}<br />
									break;<br />
								case RequestRecieve: //Request to recieve a packet<br />
									dwIoSize = 0;<br />
									iTemp = 0;<br />
									ptrSession->setStatus(AfterRecieve);<br />
									if (WSARecvFrom(ptrSession->getSocket(), <br />
										ptrSession->getWSABuffer(),<br />
										1,<br />
										&dwIoSize,<br />
										&iTemp,reinterpret_cast<sockaddr*>(ptrSession->getAddress()),<br />
										&iSockAddrSize,<br />
										ptrSession->getOverlapped(), NULL) == -1)<br />
									{<br />
										int iErrorCode = WSAGetLastError();<br />
										if (iErrorCode == ERROR_IO_PENDING){ <br />
											break;<br />
										}<br />
										else{<br />
											__REPORT("Recieving failed because: %i", WSAGetLastError());<br />
											pThis->ReleaseSession(ptrSession); <br />
										}<br />
									}<br />
								case AfterRecieve: //Call receiving event<br />
									if (dwIoSize == 1)<br />
										break;<br />
									ptrSession->incRecvBytes(dwIoSize);<br />
									if ((iTemp = ptrGameClass->UnpackServerInfo(ptrSession->getNDServer(), &objToolset)) == 0){//Session is finished!<br />
										pThis->FinishGame(ptrSession->getNDServer());<br />
									}<br />
									else if (iTemp > 0){ //coming more data<br />
										break;<br />
									}<br />
									else if (iTemp < 0){<br />
										__REPORT("%s->UnpackServerInfo() failed", ptrGameClass->Gamename());<br />
									}<br />
									pThis->ReleaseSession(ptrSession);<br />
									break;<br />
								default:<br />
									break; //Who knows?<br />
							}<br />
						}<br />
					}

Mark Salsbery30-Jan-07 13:42

30-Jan-07 13:42

Reusing the overlapped struct is no problem. You just can't use it in more than one pending
(uncompleted) overlapped operation at a time.

I suggest carefully checking error conditions here. The documented results for
GetQueuedCompletionStatus() should be handled something like this:

BOOL bIORet = GetQueuedCompletionStatus ( // Get a completed IO request
                                  pThis->m_hIOCompiltionPort,
                                  &dwIoSize,
                                  reinterpret_cast(pThis),
                                  &ptrOverlapped, INFINITE);
 
(pseudocode)
if (bIORet)
{
   if (was overlapped socket operation)
   {
      if (dwIoSize == 0)
      {
         // socket closed
      }
      else
      {
         if (AfterRecieve)
         {
            if (dwIoSize < numberofbytesrequested)
               receive (numberofbytesrequested - dwIoSize) more bytes
            else
               // all bytes received
         }
      }
   }
}
else
{
   if (ptrOverlapped)
   {
      // I/O failed on the operation associated with this completion packet
      //  GetLastError() for extended error info
   }
   else
   {
      //GetQueuedCompletionStatus() failed
      //  GetLastError() for extended error info
   }
}

Can you post the code that calls WSARecvFrom, including the parameters passed and how you are
checking for errors? Please use the "pre" tags instead of the "code" tags as well Smile | :)

Polity4h30-Jan-07 14:04

30-Jan-07 14:04

sure:

int Recieve(char* buffer, size_t buflen){
    if (m_iLastRecieved > 0){ //Were expecting a second packed
        m_wsaBuf.len = static_cast<ULONG>(buflen) - m_iTotalRecieved;
        m_wsaBuf.buf = buffer + m_iTotalRecieved;
        m_wsaBuf.buf[0] = 0;
    }
    else{
        buffer[0] = 0;
        m_wsaBuf.len = static_cast<ULONG>(buflen);
        m_wsaBuf.buf = buffer;
    }

    static int sockaddr_len = sizeof(sockaddr_in);
    DWORD temp(0);
    setStatus(RequestRecieve);
    //returning 0 is ok
    return (PostQueuedCompletionStatus(m_hIOCP, 1, reinterpret_cast<ULONG_PTR>(this), getOverlapped()) == FALSE);
}

Thats where i make my request to post. You might wonder if it aint dangerous for the buffer to overflow or something but most of the checking goes outside this function and i can assure that there is no overflow since i'm only testing with 1 game which is secured in any way i know off.

The requested bytes is unknown, it can variate depending on the used game from like 100 to a few packets of 1500 kb however mostly its just something like 1 packet with 600 bytes (i know because my previous version of the application worked perfectly, only it wasnt using IOCP just the lame one thread per server way ) anyways, with this topic still being open and all, there is one other ugly thing i cant find a good solution for. Is there any way to close a overlapped operation because i need to reuse the overlapped structures, and that means that if there is a timeout check function somewhere in my application just running once in a while over all the overlapped operations looking if they arent timed out because it could hapen that one of the 1000 servers wont give a response for some or other reason and else the overlapped structure would be in use all the time waiting for nothing to come.

Mark Salsbery30-Jan-07 14:39

30-Jan-07 14:39

Polity4h wrote:
to a few packets of 1500 kb

1500KB? That's too big for a UDP datagram Smile | :)

All I can think of is check all your send and receive counts on both ends in the debugger.
With UDP you should be receiving complete datagrams the size that were sent (providing they
arrive successfully - UDP doesn't guarantee that).

Why do you use UDP? Have you allowed for messages that don't make it?

Polity4h wrote:
Is there any way to close a overlapped operation because i need to reuse the overlapped structures, and that means that if there is a timeout check function somewhere in my application just running once in a while over all the overlapped operations looking if they arent timed out...

The only way I know is to close the socket. Any pending queued overlapped operations on the
socket will be released by GetQueuedCompletionStatus(). If you want to check periodically then
you could use a timer, the dwMilliseconds parameter to GetQueuedCompletionStatus(), or some other
method. Close the socket after the appropriate period of inactivity.

Mark Salsbery30-Jan-07 14:48

30-Jan-07 14:48

Cancel Overlapped Socket Op Update:

From a Larry Osterman (Microsoft engineer) blog, apparently you can use CancelIo(), casting your
socket handle to a "HANDLE".

Give it a try - worst that can happen is it fails Smile | :)

Mark

Polity4h1-Feb-07 9:30

1-Feb-07 9:30

CancelIo doesnt work, the thing is that it needs to be ran from the same thread where Overlapped operations were posted. I could however close the socket and open it again, but what are the drawbacks of this? this means that i need to close and open like 20 sockets per second in the future. looks really hard to me, but i cant really figure out what might happen when i do this.

Mark Salsbery1-Feb-07 10:19

1-Feb-07 10:19

Polity4h wrote:
CancelIo doesnt work, the thing is that it needs to be ran from the same thread where Overlapped operations were posted.

Yeah I thought about that since you are posting overlapped ops from your worker thread(s) Smile | :)

20 sockets per second? How long is the time you are willing to wait before the remote node is
considered inactive??

Polity4h1-Feb-07 11:08

1-Feb-07 11:08

alright, i might have thought a bid to far in the future were i got myself a mainframe Wink | ;)

<< -_- whatever, anyways, lets say 2 sockets needs to be created per second. wont this kill my system after doing this a long time ( my app needs to run constantly for, count them up a few years ) << thats the perfect vision ofcourse.

Mark Salsbery1-Feb-07 11:12

1-Feb-07 11:12

I'm not sure what you are trying to do. Why do you need to keep creating sockets but never
destroy them? Surely you'll run out of sockets if you do that Smile | :)

Polity4h1-Feb-07 11:31

1-Feb-07 11:31

...I could however close the socket and open it again, but what are the drawbacks of this? this means that i need to close and open like 20 sockets per second in the future...

hehe, anyways your saying that your server wont suffer to hard when creating AND DESTROYING lets say 2 sockets per second?

Mark Salsbery1-Feb-07 11:53

1-Feb-07 11:53

No. As long as that's really what your app needs to do. Smile | :)

Think about how many TCP sockets a busy web (HTTP) server must open and close per second.
20 per second is nothing Smile | :)

Mark

Polity4h1-Feb-07 11:54

1-Feb-07 11:54

now i know enough. thanks for everything mark, you've helped me alot

Mark Salsbery1-Feb-07 11:58

1-Feb-07 11:58

*EDIT* Having a pool of sockets could improve performance - that way only ConnectEx/DisconnectEx
needs to be called on a socket instead of recreating/destoying the entire socket every time.

Mark

Polity4h1-Feb-07 12:05

1-Feb-07 12:05

src=msdn:The Windows Sockets ConnectEx function establishes a connection to a specified socket, and optionally sends data (called connect data) once the connection is established. The ConnectEx function is only supported on connection-oriented sockets.
remember, i'm forced to use connectionless protocols Smile | :)

thanks anyway

Mark Salsbery1-Feb-07 12:12