search

ddevault / Craft.Net

(Unmaintained, see TrueCraft) Minecraft server, client, and etc for .NET
MIT License
228 stars 65 forks source link

Out Of Memory server cause by viewDistance on RemoteClient.UpdateChunks #228

Closed DotVision closed 9 years ago

DotVision commented 9 years ago

'Bug' is located on LoginHandlers.cs line 116 client.Settings.MaxViewDistance = (32 << packet.ViewDistance) + 2; Comment this line fix the issue.

ddevault commented 9 years ago

This is not a bug and you have misidentified the problem. The real bug is that chunks are never unloaded in Craft.Net and if you walk around enough you'll inevitably run out of memory. This is a known problem.

DotVision commented 9 years ago

Thanks for reply.

BTW, setting the MaxViewDistance to something HUGE, lets the viewDistance increase at each iteration and just crash the loop (both indexed on viewDistance) in the handler, way before running out of memory with the loaded chunck….

I understand perfectly the unload issue, but it seem not related..

Anyway, I was just trying to help, because my correction solved the crash issue for me and let me flying for a map of 4M chunks without problems. I will avoid myself to comment anything in the future. :)

Regards

Guillaume Pelletier | CEO | DotVision | + 33 6 07 42 26 30 | mailto:guillaume.pelletier@dotvision.com guillaume.pelletier@dotvision.com

Find us in Facebook : DotVision & DotVision Motion

De : Drew DeVault [mailto:notifications@github.com] Envoyé : mardi 6 janvier 2015 20:05 À : SirCmpwn/Craft.Net Cc : DotVision Objet : Re: [Craft.Net] Out Of Memory server cause by viewDistance on RemoteClient.UpdateChunks (#228)

This is not a bug and you have misidentified the problem. The real bug is that chunks are never unloaded in Craft.Net and if you walk around enough you'll inevitably run out of memory. This is a known problem.

— Reply to this email directly or view it on GitHub https://github.com/SirCmpwn/Craft.Net/issues/228#issuecomment-68914677 .

Aucun virus trouvé dans ce message. Analyse effectuée par AVG - www.avg.fr http://www.avg.fr Version: 2015.0.5577 / Base de données virale: 4257/8878 - Date: 06/01/2015

ddevault commented 9 years ago

A vanilla client won't set that value to anything too huge. I suppose we could add a limit to avoid malicious clients driving the server out of mem.

DotVision commented 9 years ago

My standard commercial minecraft client 7.5 has set the value to 16 (packed.ViewDistance), which is more than acceptable but this following line of code put this value as left shift operand… then the maxViewDistance is become HUGE .

32*65536 + 2= 2097154.

And as far I look, this is the only place where the MaxViewDistance is set.

client.Settings.MaxViewDistance = (32 << packet.ViewDistance) + 2

De : Drew DeVault [mailto:notifications@github.com] Envoyé : mardi 6 janvier 2015 20:33 À : SirCmpwn/Craft.Net Cc : DotVision Objet : Re: [Craft.Net] Out Of Memory server cause by viewDistance on RemoteClient.UpdateChunks (#228)

A vanilla client won't set that value to anything too huge. I suppose we could add a limit to avoid malicious clients driving the server out of mem.

— Reply to this email directly or https://github.com/SirCmpwn/Craft.Net/issues/228#issuecomment-68919518 view it on GitHub. https://github.com/notifications/beacon/ABmZNGOdVpZb76ucjVswH9hHrhIcI9JBks5nfC_sgaJpZM4DOzBs.gif

Aucun virus trouvé dans ce message. Analyse effectuée par AVG - www.avg.fr http://www.avg.fr Version: 2015.0.5577 / Base de données virale: 4257/8878 - Date: 06/01/2015

ddevault commented 9 years ago

Which version of Minecraft are you using for this?

DotVision commented 9 years ago

V8 but selected V7.5 using the minecraft launcher with last java update, on windows surface pro 3 running windows 8.1

Sent from my phone.

Le 6 janv. 2015 à 20:57, Drew DeVault notifications@github.com a écrit :

Which version of Minecraft are you using for this?

— Reply to this email directly or view it on GitHub.