World Crash - MakeApplicationPacket

[John Adams]

Now we're getting crashes in places we never had crashes before.

Stack:

Code: Select all

 	kernel32.dll!_RaiseException@16()  + 0x3c bytes	
 	EQ2World.exe!_CxxThrowException(void * pExceptionObject, const _s__ThrowInfo * pThrowInfo)  Line 157	C++
 	EQ2World.exe!operator new(unsigned int size)  Line 63 + 0x1a bytes	C++
>	EQ2World.exe!EQProtocolPacket::MakeApplicationPacket(unsigned char opcode_size)  Line 418 + 0xa bytes	C++
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 471	C++
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 485	C++
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 485	C++
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 485	C++
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 485	C++
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 485	C++
.
. (thousands)
.
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 485	C++
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 485	C++
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 485	C++
 	EQ2World.exe!EQStream::ProcessPacket(EQProtocolPacket * p)  Line 485	C++
 	The maximum number of stack frames supported by Visual Studio has been exceeded.	

Code:

Code: Select all

EQApplicationPacket *EQProtocolPacket::MakeApplicationPacket(uint8 opcode_size) const {
	EQApplicationPacket *res = new EQApplicationPacket;
	res->app_opcode_size=(opcode_size==0) ? EQApplicationPacket::default_opcode_size : opcode_size;
	if (res->app_opcode_size==1) {
==> here	res->pBuffer= new unsigned char[size+1];
		memcpy(res->pBuffer+1,pBuffer,size);

[Jabantiz]

I have seen crashes here before when I was trying to get AoD and later CoE working on the emu. Don't remember what I thought the cause was at the time, will look into it.

[Jabantiz]

It looks like new() gave an out of memory exception by looking at the call stack on EQ2TC.

Opcode for the packet it was trying to make is 574
version = 0
size = 6553
data = 0

It would seem to be a bad packet that came from he following code

Code: Select all

if(OutOfOrderpackets.find(NextInSeq) != OutOfOrderpackets.end()){
			p = OutOfOrderpackets[NextInSeq];
			if(p){
				
#ifdef EQN_DEBUG
				LogWrite(PACKET__DEBUG, 1, "Packet", "*** Processing Future packet: Seq=%i", NextInSeq);
				LogWrite(PACKET__DEBUG, 1, "Packet", "[Start]");
				//p->DumpRawHeader(NextInSeq);
				LogWrite(PACKET__DEBUG, 1, "Packet", "[End]");
#endif
				ProcessPacket(p);
				delete p;
			}
		}

so it seems to be an out of sync issue. As for how it ended up with a bad packet I am not sure if the code we use to handle it caused it or the lag or whatever else caused the desync, could use scat's opinion on this as I don't usually mess with network code.

[Scatman]

Hmm, was the size 64k or was it actually just 6553? 6553 isn't that large and there may be an issue somewhere else. Since the size integer is an unsigned short for UDP packet size, the max packet size is 65535 -IPv4 header of 20 bytes - UDP header of 8 bytes so it's actually 65507 bytes. We could check for a ridiculous size before allocating memory and drop the packet and issue a warning if it's bad. The client may get disconnected but at least world will still be running.

[Jabantiz]

Triple checked it when I was connected to EQ2TC and the size was just 6553

[John Adams]

Is it just me, or is "65535" looking a bit like "6553"?

[Jabantiz]

Is it just me, or is "65535" looking a bit like "6553"?

And that is why I tripled checked thought for sure the crash was because size = 65535 until I noticed the last 5 missing when I was checking another value.

[Jabantiz]

I have had this crash a couple times in the past couple days, this may be an invalid pointer issue as well

Code: Select all

if(OutOfOrderpackets.find(NextInSeq) != OutOfOrderpackets.end()){
    p = OutOfOrderpackets[NextInSeq]; // Get the packe from the list
    if(p){
        ...
        ProcessPacket(p);  // handle the packet
        delete p;    // delete the packet
    }
}

As you can see we get the packet, process it and delete it, but no where do we remove the packet from the OutOfOrderpackets list. I added an erase and am currently testing but this crash seems to be rare at best so it may not be a fix. Also seems unlikely that NextInSeq will ever be the same but I have no other ideas on this.

[Jabantiz]

I have had this crash a couple times in the past couple days, this may be an invalid pointer issue as well

Code: Select all

if(OutOfOrderpackets.find(NextInSeq) != OutOfOrderpackets.end()){
    p = OutOfOrderpackets[NextInSeq]; // Get the packe from the list
    if(p){
        ...
        ProcessPacket(p);  // handle the packet
        delete p;    // delete the packet
    }
}

As you can see we get the packet, process it and delete it, but no where do we remove the packet from the OutOfOrderpackets list. I added an erase and am currently testing but this crash seems to be rare at best so it may not be a fix. Also seems unlikely that NextInSeq will ever be the same but I have no other ideas on this.

[Jabantiz]

Committed my possible fix (1 line of code) from my testing I have not seen this crash again but as it seems to be rare I may have been lucky, left a char logged in by the shelf in frost fang for just over 6 hours, every now and then I did my best to desync the client (minimize the client and reopen it several times back to back) and never had a world crash or client disconnect. Again I may have been lucky so if this crash pops up again with his code let me know.