Improve handling of fake players
SpaceToad opened this issue ยท 29 comments
See http://mod-buildcraft.com/forums/index.php?topic=599.0. Anyone cares to give it a shot?
I would like to take a look at it. But i would need more details though. Would that be possible to get?
@OrionDevelopment look at the 3 crash reports in the post spacetoad linked, if you want i can get you 3 or 4 more but they are near identical, only the entity is different but the stacktrace is exactly the same
@AEnterprise I would like the stacktraces (Or are they there and I did not see them?) if that is possible. SO i can retrace some function calls as i have no way of rebuilding the setup that created the bug...
they are there, on the top most post i made, those 3 links
- crash-2014-05-11_00.49.31-server.txt (12.74 kB - downloaded 5 times.)
- crash-2014-05-11_00.54.20-server.txt (12.09 kB - downloaded 2 times.)
- crash-2014-05-11_10.07.54-server.txt (11.94 kB - downloaded 1 times.)
Hmm it might be. It is stored in the EntityPlayerMP entity cause that is were the handler itself gets the dispatcher. I will look into that tomorrow.
@OrionDevelopment yup - it's in a different folder.
Haha i fixed it before you wrote that....... (Quick spacetoad). Gradle assigns the source directories wrong. (Resources as source (which should be resource directory) and leaves the api folder alone) Luckily that is easily fixable.
Hmm looking at the code for a fix for this. On sending a packet to all players we should check if they all contain a dispatcher.. But what todo if not (Print a warning message and skip the packet)? Any ideas?
Okey looking through it seems that on the moment BC tries to updates its enities the FML Standard network dispatcher is not initialized for a certain EntityPlayerMP.Which brings me to wonder if BC is actually causing this issue. Can we confirm that this happens with only BC installed? Or if similar crashes are happening with other mods? (So without BC installed?)
@OrionDevelopment is there anyway we can check that before trying to send the message?
One other possibility could be that the fake player we create for BuildCraft causes problems? See https://github.com/BuildCraft/BuildCraft/blob/6.0.x/common/buildcraft/core/proxy/CoreProxy.java#L187-L208.
@SpaceToad that is most likely the cause, if it was a problem with players don't having a dispatcher it would have been reported on forge already and fixed, i guess the dispatcher of BC is missing it's handeler
@SpaceToad @AEnterprise yes that could be he source aswell. I will take a loop at it again this evening.
To carry on with that, let me suggest trying to replace all manual "fake" player creations in BuildCraft (such as CoreProxy.createNewPlayer) by the use of Forge's FakePlayerFactory.get. There's an issue with the "weak reference" thing to be worked out.
Thoughts?
@OrionDevelopment any news on this one?
I'm adding protection for now, but I'm not sure how good that is if it's a systematic failure. Really need some insights here.
Note that I have been completely unable to reproduce so far. If anyone has ideas...
i don't have enough knowledge of that to help with that, and i don't realy have the time to look intro that
Excellent catch @psxlover ! That looks like an extremely likely candidate indeed. Now the first release with that one in should be 1079 so we just need to tell people to upgrade. Other cleanups suggested on this ticket makes sense, but less of a priority.
@psxlover Yes that would solve this issue. Indeed a perfect catch :+1
Yeah that is probably a good idea......
I actually could not reproduce the crash myself... again just happened once while debugging the quarry thing. But I could not catch that properly
Perhaps it has something to do with MinecraftForge/FML@42713c6...503da3a
Alright, let's keep this one for the purpose of the cleanup fake players though. I'm taking care of it.
I can do that as well....
Just replace all the calls to CoreProxy.createNewPlayer with ForgePlayerFactory.get?
Nah, it's a bit more involving. You need to manage weak references, check that there's no code still relying on the client side, check other place where we create manually fake players, and so. I'm tracking all of these down. What will be very useful though is some testing once that's all in.
Alright, I believe I made proper changes, and the code should run fine. I'll try to make a quick release with that one in, but don't hesitate to test. I run most of mine in client / server mode.