Debugging 4351 client random closes.

I read the docs on beginsend and endsend. it's definitely the fact that the OS will batch packets, and if there's not enough room in the tcp/ip packet for the entire packet, endsend will return with the actual number of bytes sent, instead of starting a second tcp/ip packet to send the rest of the packet.

There are zero examples on how to actually do this that i can find. i've figured out that i can compare the result of endsend with the sendbuffersize in the callback, but i don't have a working example of how to do a second beginsend, and see no obvious way to fetch the send buffer itself, which is what's needed to actually do a second BeginSend.

I'm testing to set Sock.NoDelay to true, which should just stop this from happening, but i'd rather actually implement a real solution that allows the Nagle Algorithm to run, and actually sends the rest of the packet.

i looked at your code. it's using send, not beginsend for one, and you are most likely using it in blocking mode, so it guarantees that the entire packet gets sent.

And I had to read the MS documentation on EndSend to work out that this can happen, and that there's no way to put EndSend in blocking mode.

So the fix is to either

a) switch from beginsend+callback to send
b) disable nagle algorithm by setting NoDelay to true on the socket.
c) somehow make the callback call beginsend again to send the rest of the packet if it was only partially sent. Pretty much every single code example i can find of EndSend doesn't bother with this. It usually works fine.

I'm testing b), since it's a one line fix/ Doing a) will probably solve the issue completely, but will also lower performance, and reduce the efficiency of the nagle algorithm. Hence me asking if someone actually knows how to do C.

Hmm. putting nodelay everywhere didn't seem to fix it.

What IS the actual packet size limit for the client? I'm not seeing it in the source you linked, just a 4096 byte buffer on recieving.

I know windows will send bigger tcp/ip packets than that.

I suspect i just need to switch from BeginSend to Send everywhere in the code to actually fix it for multicore. ther are at least three DIFFERENT places.

What actually IS the size limit for conquer packets?

Okay. figured it out.

byte[] is a reference, so unless it's actually defined locally in the function (and NOT a byte[] passed in as a parameter), you MUST get a lock before passing it to another function. Of course on a single core, the issue is avoided.

So the last person left out some locks, and now i get the FUN task of looking for everywhere there actually needs to be a lock. just before doing the encryption and the send clearly isn't enough with the crazy all over the place stuff the source is using.

Quote:

Originally Posted by zaphod77 Okay. figured it out. byte[] is a reference, so unless it's actually defined locally in the function (and NOT a byte[] passed in as a parameter), you MUST get a lock before passing it to another function. Of course on a single core, the issue is avoided. So the last person left out some locks, and now i get the FUN task of looking for everywhere there actually needs to be a lock. just before doing the encryption and the send clearly isn't enough with the crazy all over the place stuff the source is using.

Single core won't fix that. If you have multiple threads being forced to running on a single core, then you can still hit a race condition on a resource when there's a context switch between threads. It may reduce the chance of that happening by reducing the performance of the server, but it won't eliminate it.

Regarding your send... you already have a copy and lock. If anything, the copy is in the wrong place, but adding more locks can be dangerous if you don't know what you're doing...

the bit for small packets was the issue.

it messes about with yy, which is a MemoryStream. while the lock does exist when it writes to the stream, the YY.Elapsed didn't request a lock.

Quote:

void YY_Elapsed(object sender, ElapsedEventArgs e) { YY.Stop(); byte[] packet = Senddata_s.Getpacket(); Crypto.Encrypt(ref packet); try { if (Sock != null) { if (asyncCallback == null) { asyncCallback = new AsyncCallback(OnSendCallback); } Sock.BeginSend(packet, 0, packet.Length, System.Net.Sockets.SocketFlags.None, asyncCallback, Sock); } } catch (Exception c) { Console.WriteLine(c); } }

Of course that was back when i was using BeginSend, which as MS says can't be trusted to block till it sends the full packet.

And it can copy stuff out of that MemoryStream while it's being written, leading to an incomplete packet. I haven't had the crash since i added the additional locking.

Also, i noticed that the timer wasn't being stopped while the data was being streamed into the memorystream.

Quote:

{ Senddata_s.Adddata(data); if (YY != null || YY.Enabled == true) { YY.Stop(); }

so i fixed that too. now it stops the timer, THEN streams the packet in, THEN restarts it. Which was probably the actual problem, and i probably have an extra lock somewhere now, but it's not crashing my client in testing anymore, so I'm happy.

And at least on my windows, hyperthreading shows up as a core in the affinitymask.

Quote:

Originally Posted by zaphod77 And at least on my windows, hyperthreading shows up as a core in the affinitymask.

Hyperthreading is a bad name for what it does, because a virtual core is not the same as a single thread. If it were, then your computer would only be able to run 1 program per virtual core, max. In reality, your computer runs hundreds of processes at a given time with thousands of threads of execution. Each thread takes priority on the CPU, and the scheduler in the OS will switch between thread contexts that it's executing per virtual core. So your server can still be multithreaded without executing on multiple cores.

Quote:

Originally Posted by zaphod77 it messes about with yy, which is a MemoryStream. while the lock does exist when it writes to the stream, the YY.Elapsed didn't request a lock.

I know you're not asking for advice, and dear god do you not listen to it, but I'd really recommend against naming methods things like "YY", because that doesn't mean anything to someone trying to read it (including yourself in 3-4 months, probably). Obfuscating your own code only hurts yourself. I'd really recommend using more meaningful names.

lol i wasn't the one who named it.

you think that's obfuscated?

I finally got so annoyed with one of the variablenames there that i had to search and replace it with "DragonGems" globally.

You think I would have done stuff like .YY and _bao and zjmfgj and zjwlmz (those last two are phoenix gem percentage, and something to do with crits, not that you'd know that... i suspect they are chinese abbreviations)

Quote:

Originally Posted by zaphod77 lol i wasn't the one who named it. you think that's obfuscated? I finally got so annoyed with one of the variablenames there that i had to search and replace it with "DragonGems" globally. You think I would have done stuff like .YY and _bao and zjmfgj and zjwlmz (those last two are phoenix gem percentage, and something to do with crits, not that you'd know that... i suspect they are chinese abbreviations)

I don't know you. I don't know what you wrote or what you're adding. I'm just reading code and the information you're posting.

fair enough. that wasn't an attack at all. i'm only messing with this insane source because some wackos want to play it again. already made a number of improvements, some of which i could have sworn i had already done last time i had the source.

I know a lot of stuff is just flat out wrong, but that's how it was, and i don't know enough about the original game to make corrections.

I'd much rather run something that was actually like Qonquer was. It had the best metzone of any private server i've seen. Hunting the TreasureBirds with an archer that wasn't high enough level to drop items on death for nice numbers of meteors was FUN.