Kademlia

Confirmed for 1.13-pre1 see latest image

See https://hub.spigotmc.org/jira/browse/SPIGOT-1097

worldserver.flushSave(); is called by CommandSaveAll and results in a syncronous call to the mentioned method.
This can result in the creation of two iterators which are not thread safe even when using a ConcurrentHashMap and can result in at least a NPE on .next().

As far as i can tell this was fixed with https://bugs.mojang.com/browse/MC-46345
Every version above 16w42a should be fixed currently. Thus 1.10.2 should still have the problem.

MC-46345 was marked as fixed, if this is actually the case (no version released yet) this should fix MC-80782 and MC-98707 too.
If MC-46345 somehow does not fix the other two the severity of the memoryleak may increase greatly.

Possibly caused by MC-10976. The bug described there was already present in minecraft 1.4.x

As mentioned in point 4, yes it is.
Edit: I´d argue a new topic with all information is worth it, as this is neither only a single-player problem as the title suggests, nor does this only happen when quitting the game.

I understand what you are trying to say but this is still not very helpful to the ticket:

Your viewing the network/memory cost from the wrong side. By default all Sections are transferred (100% usage). Sometimes there are sections not transferred because of this faulty code (they should be anyways and everyone expects them to!). If you take typical values, lets say a viewdistance of 6 and and 5 sections each chunk. Thats 169 Chunks or 845 Sections. A missing section because of this bug is about 0,1%. This concern is not justified and even a recommendation to implement something to save 0,1% is a pretty hard sell.

I do not understand why you wrote this.

• MCP Naming is not a concern here

• The network cost is already explained in the conclusion. Your elaboration adds nothing new.

• Your remark regarding a null-Section of a chunk is wrong. The code is built around identification of null-Sections.

• Your recommendation adds work on the server, the client and can have complicated new validation cases. For a problem of this magnitude this is absolutely over engineering. I have already stated that these changes are validated and working for over two years now. There is no need to complicate the issue buy adding unnecessary additional work.

• The whole idea behind the isEmpty-Check is faulty. It only hides information. Removing it is the ideal solution.

Confirmed 1.10; same seed same coordinates

Confirmed for 1.10 😞

Hey, thank you for responding. Showing this via a worldmap would be quite difficult. I hope relating to the code helps:

This is based on the currently available minecraft_server.1.9.4.jar (and deopfuscated spigot codes)
Obfuscated:
Class: aht.java
Method (~Line 426): public int c(cl \u2603, boolean \u2603)

Deobfuscated:
Class: World.java
Method ( ~Line: 623): public int c(BlockPosition blockposition, boolean flag)

This method would be something like getMaxLightLevel(Position position, boolean firstCall).
The method will try to get the highest light level. It returns the light-level of the block itself if it has one (air/non-solid block)
or the highest of the five closest blocks (up,north,east,south,west). This second part has the problematic code.

Methodflow should be:

• Return 15 (maxLight) if invalid position

• if(firstCall && getBlockType(position).isSolid())

  • recursive call to self with neighbor1-block and firstCall = false

  • recursive call to self with neighbor2-block and firstCall = false

  • recursive call to self with neighbor3-block and firstCall = false

  • recursive call to self with neighbor4-block and firstCall = false

  • recursive call to self with neighbor5-block and firstCall = false

  • return highestValue of those calls

• *else*

  • *Chunk c = getChunkAt(position)* (this is called for all 5 recursive calls from above and results in a forced-chunk-load if the chunk is not yet loaded)

  • return c.getLightLevel(position)

Chunkloads based on this code will happen every time the growth-algorithm tries to grow a plant at the border of a players visible (loaded) area.

Adding something like this code before the highlighted chunk-loading would fix this

if(!isLoaded(position)){ //kade edit; do not load a chunk just to look at the light-value
    	return 0;
   }

On a side-note the same happens when trying to spread grass, mushrooms, mycel, cocoa etc.
In these files there are checks to get the blocktypes of the surrounding blocks and try to spread these blocks afterwards.
Those calls force sync-chunkloads too.

Let me know if there is something missing or additional information are needed.

Sorry I dont understand you comment. None of your reports are even related to client-side memoryleaks and maps. For this Bug it does not matter what server you join. Just join random Servers with maps for some time and your client will crash at some point.

A player in a team with collision disabled
(/scoreboard teams option a collisionRule pushOwnTeam)
will still be able to push teammates if riding a (horse) vehicle.
This seems to be a result of the client-sided logic of the player being pushed.

This issue is not fixed in 1.9.2.
https://www.youtube.com/watch?v=biQF9qu2ww8

MC-46345 only refers to the maps no longer updating.
This report refers to the memory never being cleared and ultimately causing a OutOfMemoryError.

Here is a way of easy reproduction with any client and any server:

1. Increase your lookup-time for a servers DNS/IP (see start of the video)
(This can be done by using a proxyserver and forcing DNS-Lookups via the proxy)
2. Use "REFRESH" to mark all servers as "unknown" for a little while
3. Connect to two servers shortly after another.

Video:
https://www.youtube.com/watch?v=OpsrZB0x17I

• Refreshing to show the resolve-latency

• Connecting with a little delay (second connect 200ms later)

• Visibly connection to two servers shortly after another

• Flickering sky as one server has night and one has daytime

• Commands upto 2015-06-11 00:18:13 are only registered on Server 1

• "/chunk vd 5" forces a world-Change-Event with a 5-chunk viewdistance this forces a disconnect somehow from server 1

• All additionall commands are registered by Server 2

• TAB-Shows other players

Serverlog Server 1:

[2015-06-11 00:17:37 INFO]: Kademlia[5] logged in with entity id 5850 at ([world]920.5143804149387, 68.0, 2184.500539235966)
[2015-06-11 00:17:38 WARN]: Kademlia moved too quickly! -904.3122679052309,9.0,-1787.1306913904555 (904.3122679052309, 9.0, 1787.1306913904555)
[2015-06-11 00:17:52 INFO]: Kademlia issued server command: /v
[2015-06-11 00:17:54 INFO]: Kademlia issued server command: /lag
[2015-06-11 00:17:56 INFO]: Kademlia issued server command: /list
[2015-06-11 00:18:04 INFO]: Kademlia issued server command: /w spawn
[2015-06-11 00:18:13 INFO]: Kademlia issued server command: /chunk vd 5
[2015-06-11 00:18:16 INFO]: Saved 1 players in 1 MS.
[2015-06-11 00:19:01 INFO]: Kademlia lost connection: Disconnected

Serverlog Server 2:

2015-06-11 00:17:38] [Server thread/INFO]: Kademlia[1] logged in with entity id 5997165 at ([world]16.202112509707884, 77.0, 397.36984784551026)
[2015-06-11 00:18:14] [Server thread/WARN]: Kademlia moved too quickly! -126.60349590885434,-12.0,249.28627572675515 
[2015-06-11 00:18:25] [Server thread/INFO]: Kademlia issued server command: /chunk vd 15
[2015-06-11 00:18:40] [Server thread/INFO]: Kademlia issued server command: /w spawn

@Jarcode im talking about witholding the packet. The server will change the player but not inform the player about that change.
This results in various client-side bugs/glitches but the maps are still working as intended on the client side. Meaning the packet has this bug as a direct result - no other packets should need to be checked to fix this bug.

OT: A tip for your plugin: if you intend to support bungeecord you will need to split the MapId-Cache into parts and give all servers a section - else the players will get bugged maps once changing the servers - had the same problem.

I did some further testing. If a Client does not receive the "PacketPlayOutRespawn" packet this bug will not occur. The player will as a result experience various other bugs but i could confirm that this bug is specifically triggered by the PacketPlayOutRespawn Packet.