Negative map IDs causes memory leak when updating ItemFrames

I have no issue creating a test-case plugin, or exploring the command API to see if this is capable of being reproduced in more than one way.

I took the time to implement this same effect using only command blocks. The memory leak does not occur. I feel like this could be an issue with the way information is sent to the client for changing the information. If I find that it is an issue with an unchecked variable with the metadata packet sent to the item frames, I'll post an update here because the client can still leak memory really bad from the original method used that still causes a leak.

^{The button on the far right clears the board entirely. The one to the left creates it, and the lever toggles the loop to cycle map changes.}

I have done more testing writing a test-case using Spigot's plug-in API. I have tested using ONLY the basic API for Spigot and found no leaks.

I have also included the packets that I have sent to the player manually using the NMS API, and I did find out what exactly is causing this memory leak. It's due to negative Map ID's. If I set my map IDs to negative and constantly cycle ImageFrames, The client-side memory allocation goes up very quickly. (16x9 frames cycling every half second between 2 maps) If I set the map IDs to a positive integer, the memory leak vanishes.

I can package up the plug-in code as a test-case on GitHub, however it seems to be directly caused by negative ID's.

I have tested further with the negative IDs using the existing commandblock API test, modified to use negative map IDs. This causes the client-side memory leak, independent of any server-side modification.

^{The command block commands I used are:}

CLEAR:

kill @e[type=minecraft:item_frame,x=180,y=70,z=60,dx=16,dy=9,dz=1]
fill 180 70 55 196 79 62 minecraft:air

CREATE:

fill 180 70 60 196 79 60 minecraft:stone
fill 180 70 59 196 79 59 minecraft:command_block{Command:"summon minecraft:item_frame ~0 ~0 ~2 {Facing:3,Item:{id:\"minecraft:filled_map\",tag:{map:1}}}"}
fill 180 70 58 196 79 58 minecraft:redstone_block
fill 180 70 56 196 79 56 minecraft:command_block{Command:"data merge entity @e[type=minecraft:item_frame,limit=1,dz=5] {Item: {id:\"minecraft:filled_map\", Count:1b, tag:{map:-1}}}"}
fill 180 70 55 196 79 55 minecraft:redstone_block
fill 180 70 59 196 79 59 minecraft:air
fill 180 70 59 196 79 59 minecraft:command_block{Command:"data merge entity @e[type=minecraft:item_frame,limit=1,dz=2] {Item: {id:\"minecraft:filled_map\", Count:1b, tag:{map:-2}}}"}

LOOP:

1

fill 180 70 58 196 79 58 minecraft:air
fill 180 70 55 196 79 55 minecraft:redstone_block

2

fill 180 70 55 196 79 55 minecraft:air
fill 180 70 58 196 79 58 minecraft:redstone_block

I'll try to reproduce this some time tomorrow. Sounds like a valid bug, though negative map IDs are a thing that don't occur normally so I'm not sure how meaningful it is (normally maps are ID'd by map_#.dat, and I don't think that number will ever be negative). So probably some code related to that is causing an issue, though I'm not sure what's actually parsing the number nowadays.

@Marcono1234, Was that test case of any use?

Thank you very much and sorry for the additional trouble. I mainly wanted to see whether Spigot exposes methods to cause this problem.

It appears [MapMeta.setMapId​(int id)|https://hub.spigotmc.org/javadocs/spigot/org/bukkit/inventory/meta/MapMeta.html#setMapId-int-] is a little bit problematic. Maybe the documentation should warn that using not assigned map IDs can cause problems, or using them should throw exceptions. Maybe the method should not even exist but instead something like setMapView should be provided to prevent using arbitrary IDs.

Could you please report this bug to Spigot?

I am resolving this report as "Invalid" now.

@Marcono1234, I can report that, sure. I'm curious though as to the nature of why memory built up as a result of using negative integers. Is there a check on the client's end for this? What's actually going on? I'm still seeing a memory leak as a cause of this. I don't expect a leak (even with improper data values for IDs), as a purposeful design. The same goes with the CommandBlock API.

Thanks.

I think it is a combination of the problem mentioned before (negative ID resulting in new map IDs being created all the time) and that it is likely more expensive for the client to handle the maps than for the server since it stores rendering related data as well.
Though I have not analyzed the memory after using the reproduction commands.
If I recall correctly references to the map data and the rendering data are kept until the player leaves the world.

If the problem is really only caused by the negative IDs, then I think this is the intended behavior.

The Spigot API was changed to prevent using unassigned IDs, see SPIGOT-4608.