MC-79154Hoppers occasionally duplicate items
Server duplicates items in hoppers. After a while, all hopper clocks are getting broken, because there is more than one item in it. I managed to duplicate diamond blocks.
Server Console says:
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:02:23] [Server thread/WARN]: Can't keep up! Did the system time change, or is the server overloaded? Running 2144ms behind, skipping 42 tick(s)
Edit: I now caught it happening. A hopper speed up so fast, that even a comparator doesn't work anymore. I'm sure this leads to item duplication.
Code analisys by Timothy Miller can be found in this comment.
Linked Issues
is duplicated by1
Attachments1
Created Issue:
Item Duplication [Server]
Server duplicates items in hoppers. After a while, all hopper clocks are getting broken, because there is more than one item in it. I managed to duplicate diamond blocks.
Server Console says:
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:02:23] [Server thread/WARN]: Can't keep up! Did the system time change, or is the server overloaded? Running 2144ms behind, skipping 42 tick(s)Environment
Linux
AMD64
OpenJDK
relates to
Server duplicates items in hoppers. After a while, all hopper clocks are getting broken, because there is more than one item in it. I managed to duplicate diamond blocks.
Server Console says:
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:02:23] [Server thread/WARN]: Can't keep up! Did the system time change, or is the server overloaded? Running 2144ms behind, skipping 42 tick(s)Edit: I now caught it happening. A hopper speed up so fast, that even a comparator doesn't work anymore. I'm sure this leads to item duplication.
Server duplicates items in hoppers. After a while, all hopper clocks are getting broken, because there is more than one item in it. I managed to duplicate diamond blocks.
Server Console says:
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity
[13:02:23] [Server thread/WARN]: Can't keep up! Did the system time change, or is the server overloaded? Running 2144ms behind, skipping 42 tick(s)Edit: I now caught it happening. A hopper speed up so fast, that even a comparator doesn't work anymore. I'm sure this leads to item duplication.
Server duplicates items in hoppers. After a while, all hopper clocks are getting broken, because there is more than one item in it. I managed to duplicate diamond blocks.
Server Console says:
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:02:23] [Server thread/WARN]: Can't keep up! Did the system time change, or is the server overloaded? Running 2144ms behind, skipping 42 tick(s)Edit: I now caught it happening. A hopper speed up so fast, that even a comparator doesn't work anymore. I'm sure this leads to item duplication.
Server duplicates items in hoppers. After a while, all hopper clocks are getting broken, because there is more than one item in it. I managed to duplicate diamond blocks.
Server Console says:
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:02:23] [Server thread/WARN]: Can't keep up! Did the system time change, or is the server overloaded? Running 2144ms behind, skipping 42 tick(s)Edit: I now caught it happening. A hopper speed up so fast, that even a comparator doesn't work anymore. I'm sure this leads to item duplication.
Code analisys by Timothy Miller can be found in this comment
Linux
AMD64
OpenJDK
Item Duplication [Server]Hoppers occasionally duplicate items
![[Mod] Neko](../../../data/avatars/users/92901.png){kind=small}
Server duplicates items in hoppers. After a while, all hopper clocks are getting broken, because there is more than one item in it. I managed to duplicate diamond blocks.
Server Console says:
[13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:01:44] [Server thread/WARN]: Fetching addPacket for removed entity [13:02:23] [Server thread/WARN]: Can't keep up! Did the system time change, or is the server overloaded? Running 2144ms behind, skipping 42 tick(s)Edit: I now caught it happening. A hopper speed up so fast, that even a comparator doesn't work anymore. I'm sure this leads to item duplication.
Code analisys by Timothy Miller can be found in this comment.
relates to
![[Mod] Pokechu22](../../../data/avatars/users/92931.png){kind=small}
relates to
is duplicated by
relates to
relates to
blocks
MC-15019
is duplicated by
blocks
In the process of testing the fix for MC-79154, I uncovered another potentially serious but unrelated Minecraft bug. I found it by looking at MCP for 1.11.2, although I'M SURE it affects 1.12.x also.
Here's what I saw in my logs:
// On tick 5034266, a chunk unload happens, causing a hopper I was monitoring to get serialized as NBT data and queued for I/O:
Hopper save on tick 5034266 ser=29333 oser=7
Position at co{x=-1, y=56, z=2}
data: {TransferCooldown:3,x:-1,y:56,z:2,Items:[],id:"minecraft:hopper",serial_num:7L,Lock:""}
// Two game ticks later, the same chunk is reloaded, but we get completely different data:
Hopper load on tick 5034268 ser=29341 oser=7
Position at co{x=-1, y=56, z=2}
data: {TransferCooldown:7,x:-1,y:56,z:2,Items:[0:{Slot:0b,id:"minecraft:redstone",Count:1b,Damage:0s}],id:"minecraft:hopper",serial_num:7L,Lock:""}
// Searching backwards in the log, we find that this is what was written out the previous time this chunk was unloaded:
Hopper save on tick 5034170 ser=29323 oser=7
Position at co{x=-1, y=56, z=2}
data: {TransferCooldown:7,x:-1,y:56,z:2,Items:[0:{Slot:0b,id:"minecraft:redstone",Count:1b,Damage:0s}],id:"minecraft:hopper",serial_num:7L,Lock:""}
(In case you're wondering why the saves are only 96 ticks apart, I think it's because the one on tick 5034170 happened when I logged out or restarted the server. I forget exactly, but it appears I triggered an off-schedule auto-save. This is actually relevant to Rich Crosby's fix for MC-22147. Briefly, I thought it might be moot, since auto-saves are 45 seconds apart. What are the chances that the I/O system would be backlogged by 45 seconds, triggering that bug? But anything that causes two auto-saves to occur close to each other CAN trigger that bug.)
Anyhow, so, how could we be loading an old version of this data? Well, here's what I found:
There is a time between when AnvilChunkLoader.writeNextIO dequeues a chunk from chunksToRemove and when the I/O write completes when AnvilChunkLoader.loadChunk could come along looking for the same chunk. If that happens, AnvilChunkLoader.loadChunk will load a stale version of the chunk.
The key methods on RegionFile are synchronized, so I'm pretty sure it can't read corrupt data. Sector allocation and low-level file access are all synchronized. The problem is what happens in the period of time when CompressedStreamTools and the various NBTTag objects are are writing out the NBT data to the ChunkBuffer (derived from ByteArrayOutputStream), including the overhead of doing the compression.
I don't have an "easy one-line" fix for this.
Let's say that AnvilChunkLoader.writeNextIO were to pick a chunk out of the chunksToRemove but not actually remove it form that container during I/O. That would be great for the case when the same chunk is reloaded. But what if another save for the same chunk comes along during that time? Then the newer one would replace the older one in chunksToRemove while the older one is being saved, and then when the write was done, the newer one would get deleted from chunksToRemove.
That condition could be eliminated (fixing Crosby's observation in the process) if an auto-save can't even be started if AnvilChunkLoader's I/O queue is not empty.
How to reproduce: Point two hoppers into each other on a chunk border, throw one item in, create lag.
Some interesting new observations. I'm not confident enough to say anything definitive at this time, but here's what I think right now from my testing and code analysis:
- The symptoms of
MC-22147are actually caused by the same block entity bug I described inMC-79154. - Rich Crosby's fix addresses a real bug, but that bug is not what causes
MC-22147. However, a good fix forMC-119971should fix Crosby's bug also.
Update: A while ago, I had put a debug message in AnvilChunkLoader that would get printed out if a chunk was being saved but got thrown away because of pendingAnvilChunksCoordinates. I just saw it print out for the first time after having done all kinda of extensive testing. Although rare, data loss CAN happen. The fix I'm working on for MC-119971 does indeed address this.
It is very likely that this is caused by a combo of MC-79154 and MC-119971.
MC-79154 pertains to block entities being momentarily having duplicates, so sometimes two of the same hopper at the same position will push the same item into another hopper twice.
Well, blocks being pushed by pistons are block entities while they are moving. So when Gnembon tried out my fix for MC-79154, he and MethodZz observed improvements for some piston-related block deletions that occurred in chunks at the edge of view distance. In my testing, I saw some block duplication problems clear up as well.
But then I also saw some deletions, both of items in hoppers and blocks being pushed by pistons. That's MC-119971, and I'm testing a fix for that right now. Incidentally, that test also subsumes Rich Crosby's suggested fix for MC-22147, although his fix doesn't actually fix MC-22147.
I've uploaded the source code that fixes this for me. I have a setup that will duplicate and delete items in hoppers and blocks being pushed by pistons, when using an unmodified server. MC-79154 fixes duping within chunks, while MC-119971 fixes problems at chunk boundaries. Together, all of these problems appear to go away.
Most of the code changes are comments. What I did was make the synchronization explicit for maps that keep track of pending chunks. One map is for those queued to be written, and another is for what's being currently written. Transitions between them, chunks entering and leaving the system, and peeking into them for reloading all have to be synchronized. Only by eliminating the race conditions can we be sure there will be no data loss.
I'll get some friends to test this further, but I think I've managed to find proper fixes to some bugs that have annoyed technical Minecraft users for a long time. What is necessary to get the developers to notice these fixes and implement them?
The fix of MC-79154 should also fix this.
Thanks @devs and @mods for getting this fixed!
We can now rely on hopper clocks not breaking, as long as they don't straddle a chunk boundary!!
However, if hopper pairs are put on a chunk boundary, it's still really easy to get them to dupe items, so we may want to think a little bit about whether or not the bug-as-originally-reported is really fixed, given that it's still possible to make hoppers dupe items. One of the causes is fixed by applying the fix attached to MC-79154 (which is a HUGE improvement, so thank you), but another cause isn't. The fix for the chunk boundary case is attached to MC-119971.
Long hopper lines for item delivery may still experience some problems, and pistons pushing blocks across chunk boundaries definitely will. Those are also fixed by the solution attached to MC-119971. In fact, the fix attached to MC-119971 is likely to also fix entity duplication and deletion that sometimes occurs at chunk boundaries. That fix is also being added to carpetmod so that community members can test it further.
Hi, Matti,
Reading over what you said, I cannot find anything to disagree with. It sounds just like my reasoning for my implementation.
Anyhow, now we should wait for the devs to get involved and not give them too many comments to read. It may be useful to mention this bug on social media, but very politely, because I hear that the devs are planning to fix MC-79154, which I am super-excited about. We need to let people know that the devs have been listening – bugs like MC-119971 are game-breaking, while things like random ticks on liquids was merely inconvenient yet they were kind enough to implement a change due to user request.
Thanks.
Kademila,
I had planed to go into more detail on lots of possible solutions on like /r/mojira or something. We did think of this, of course. To address your concern, I'll cover a few options here, but in brief. Since I'm doing this from memory, I know I'm missing a number of solutions we had considered. Xcom can add some more if he wants.
Note that the growing up problem is separate. That is an explicit event that can be handled better exactly when it happens. I believe Xcom's solution to that is to compute a motion vector that would simply move the entity the right distance away from blocks that it is going to intersect.
- Saving and loading the hitbox:
On load, this would restore the exact state prior to the save. This can be done in a backwards and forwards compatible way too. But if the standard hitbox size changes, then the hitbox won't change for pre-existing entities, which may at least temporarily miss out on bug fixes (there have been many instances of hitbox bugs, along with intentional changes to entity hitbox sizes).
- Move entities only based on coordinates, where hitboxes are always computed on demand:
This would substantially complicate the code that handles motion that must account for hitboxes. It also doesn't address your concern.
- Quantize or otherwise carefully compute coordinates and hitbox values so that conversion each way is always lossless.
This fixes the save and load problem, but it complicates coordinate and hitbox calculations in ways that are hard to understand. It also doesn't address your concern.
- When converting from hitbox to coordinates, provide hints to that conversion function that make it ensure that when coordinates are later converted back to hitbox that the new hitbox will respect the given boundaries.
It's really not that complicated how I would handle this, but it also doesn't address your concern.
- When computing motion vectors, restricted by blocks, add a margin to avoid later rounding errors:
This is Xcom's fix. It's really simple, requiring minimal code changes. However, it doesn't address your concern.
- On reload, check entities for overlap with blocks and push them out if necessary:
This works well and would handle both rounding error and the case you mention of the hitbox growing due to a code change. At the same time, this would break things that people rely on, like encasing a mob inside glass blocks.
- Keep track of any block boundaries that affect the entity's position when it's saved. On reload, ensure those boundaries are still respected.
This would require that we save both the prior hitbox and a bit vector indicating which hotbox faces must be enforced. On reload, if there are no such constraints, a new hitbox will be computed as usual, handling the case where the standard hitbox size changes. If there is a boundary, then the entity can be pushed away from the blocks to ensure that the boundary is respected, handling both size changes and rounding artifacts. If the entity is supposed to be overlapping a block (like encasing a mob in glass), then no such constraint will be recorded, allowing the size to change without undesirably pushing it out of the block. This solution would also slightly simplify the code for handling the growing up case.
That last case seems like the ideal solution, doesn't it? And if I were a Mojang employee, that is exactly what I would implement. However, this solution requires a lot of changes and is difficult to explain. We could post code, but Mojang employees are hamstrung by an idiotic Microsoft policy that forbids them to even looking at code we post here. It's short-sighed meddling-from-the-higher-ups that game-breaking bugs like MC-119971 may never get fixed, because Mojang employees don't want to get fired. As a result, you will probably always have problems losing and duping entities, blocks, and items at chunk boundaries. At least until I can get Mojang to offer me a job to fix bugs, ha-ha.
So what we decided to do is suggest the simplest solution that would fix all of the most common cases. Implement this, and all mobs will survive on saving and reloading. Implement Xcom's other fix, and all mobs growing up will also survive. The only case that isn't covered is when hitbox sizes change between saving and reloading, which only happens on version upgrades, and this would not be the first time inconveniences happened on version updates. I think we can tolerate having that corner case unfixed for a while longer. If we insist that every possible scenario be handled, we'll never get anything fixed. If Mojang had this level of perfectionism, then they would indeed have fixed MC-119971 when they fixed MC-79154. That obviously didn't happen, and so hopper duping isn't 100% fixed. Practical compromises have to be made.
And to be sure, "mobs sometimes glitch into walls if their hitbox size changes as a result of a version update" is not part of this bug report. In fact, I bet you've never seen it happen! If you're really that hot and bothered by this remote corner case, we can file it as a separate bug report (actually, I think we should in any case).
If the world was upgraded from prior to 1.12.2, then MC-79154 would create duplicate block entities, although in my experiments, it was always for only part of a tick. There may be some other bug that saves multiple copies of block entity data with a chunk, perhaps. Upgrading from pre-1.12.2 would need to account for how 1.12.2 had dealt with which duplicate was shown. With chunk borders being involved, MC-119971 and MC-108469 might be related.
After having this issue:
Please force a crash by pressing F3 + C for 10 seconds while in-game and attach the crash report (minecraft/crash-reports/crash-<DATE>-client.txt) here.
This is a payed 14/7 Server, I will try my best to catch the issue happening.
Are you sure it is client releated? Also I've found this Bug-Report: (
MC-78155) In the comments people say this speed up could cause duplication.Log of forced crash
I see this bug in chunks charged with redstone and lazychunks.
Can confirm this. We had a lot of hopper pairs facing into each other across chunk borders on or SMP server, in an attempt to create chunk loaders. We had a single item of cobblestone oscillating in each hopper pair. Upon inspection after a few weeks, most hoppers had 2-4 stacks of cobblestone. I first thought those had been left there on accident, when digging the tunnels, but turns out this was the item duplication bug. Then I recreated the setup in our base, with 3 pairs of hoppers, two of them at a chunk border, with different items, one item per pair. All three had about 4-7 items in each hopper, overnight.
I'm not sure whether it has something to do with the server messages "[Server thread/WARN]: Fetching addPacket for removed entity". I had some of these, too, but only 3 that night. One other player was logged into the server at that time.
I can confirm that this happens in 15w45a in SMP.
I do not know if this is related to this, but in Minecraft 1.8 (decompiled using MCP) the server would still send packets for dead entities. It should probably rather not send packets in this case.
This returned null needs to be tested for later on in the updatePlayerEntity(EntityPlayerMP p_73117_1_) method of the net.minecraft.entity.EntityTrackerEntry class.
Affects 1.9
Is this still an issue in the most recent versions (currently that is 1.10.2, or 16w42a) of Minecraft? If so, please update the affected versions and help us keeping this ticket updated from time to time. If you are the owner/reporter of this ticket, you can modify the affected version(s) yourself.
At the chance of "necroing" this: It was always an issue and still is (1.11.2 for sure, snapshots extremely likely but would need to test)
Doing experiments with MCP, I've figured out why hoppers are duping items.
My experiments are based on what I saw in Gnembon's video (https://www.youtube.com/watch?v=q6r6lfc6o0o), and I can reliably get items to dupe by causing chunks containing hopper pairs to load and unload over and over again. Because the conditions are controlled, I could keep a rolling log of relevant activity and then dump it out the instant that a hopper pair was detected with more than one item between them. Below is excerpts from a log that demonstrates the problem happening. (Markdown makes some of the pastes look wrong.)
// We get a chunk unloading on game tick 1363398. But it's actually tick 1363399 because of when worldTotalTime is incremented. unloadQueuedChunks 1363398 ... // And the 587th TileEntityHopper object created is among them (remember, this is really tick 1363399) Hopper save on tick 1363398 ser=587 oser=3 Position at co{x=-8, y=56, z=7} data: {TransferCooldown:1,x:-8,y:56,z:7,Items:[0:{Slot:0b,id:"minecraft:redstone",Count:1b,Damage:0s}],id:"minecraft:hopper",serial_num:3L,Lock:""} ... // On the very same tick, that chunk is reloaded, with the hopper having a new identity as the 593rd TileEntityHopper object. Hopper load on tick 1363399 ser=593 oser=3 Position at co{x=-8, y=56, z=7} data: {TransferCooldown:1,x:-8,y:56,z:7,Items:[0:{Slot:0b,id:"minecraft:redstone",Count:1b,Damage:0s}],id:"minecraft:hopper",serial_num:3L,Lock:""} ... // But for some reason, #587 still exists in worldserver.tickableTileEntities. Coincidentally, its cooldown timer runs out, so it gets ticked, transferring an item to its neighboring hopper. Hopper transfer on tick 1363399 ser=587 oser=3 Source at co{x=-8, y=56, z=7} prev: {TransferCooldown:1,x:-8,y:56,z:7,Items:[0:{Slot:0b,id:"minecraft:redstone",Count:1b,Damage:0s}],id:"minecraft:hopper",serial_num:3L,Lock:""} next: {TransferCooldown:8,x:-8,y:56,z:7,Items:[],id:"minecraft:hopper",serial_num:3L,Lock:""} Target at co{x=-8, y=56, z=8} prev: {TransferCooldown:2,x:-8,y:56,z:8,Items:[],id:"minecraft:hopper",serial_num:4L,Lock:""} next: {TransferCooldown:8,x:-8,y:56,z:8,Items:[0:{Slot:0b,id:"minecraft:redstone",Count:1b,Damage:0s}],id:"minecraft:hopper",serial_num:4L,Lock:""} ... // Finally, #587 is removed from the list: tickableTileEntities.removeAll 1363399 ... // Subsequently the NEW hopper at these coordinates is ticked, adding an extra item to the neighboring hopper. Hopper transfer on tick 1363400 ser=593 oser=3 Source at co{x=-8, y=56, z=7} prev: {TransferCooldown:1,x:-8,y:56,z:7,Items:[0:{Slot:0b,id:"minecraft:redstone",Count:1b,Damage:0s}],id:"minecraft:hopper",serial_num:3L,Lock:""} next: {TransferCooldown:8,x:-8,y:56,z:7,Items:[],id:"minecraft:hopper",serial_num:3L,Lock:""} Target at co{x=-8, y=56, z=8} prev: {TransferCooldown:8,x:-8,y:56,z:8,Items:[0:{Slot:0b,id:"minecraft:redstone",Count:1b,Damage:0s}],id:"minecraft:hopper",serial_num:4L,Lock:""} next: {TransferCooldown:8,x:-8,y:56,z:8,Items:[0:{Slot:0b,id:"minecraft:redstone",Count:2b,Damage:0s}],id:"minecraft:hopper",serial_num:4L,Lock:""} ...The trick is to get a chunk to unload, reload, and have a hopper's cooldown timer expire, all on the same game tick.
Here's why this goes wrong:
this.worldInfo.setWorldTotalTime(this.worldInfo.getWorldTotalTime() + 1L);
This ordering is wrong. Any tile entity that is scheduled to be removed should NOT get ticked.
The SOLUTION is to move this in World.updateEntities:
To above this:
I'll be testing this over night to make sure it works. Also, we really need to look around for any other such ordering reversals.
Indeed this works! I was initially suspecting some more crazy stuff causing it, but the hopper code seemed to be clean, and the actual reason happened to be more trivial than everybody suspected. since tileEntitiesToBeRemoved is only populated via calls from chunk unloading, clearing this list before entities get processed makes sense, and I don't see where this might cause other issues. I was always reluctant in moving stuff around and how they are executed within the tick, but this change makes absolute sense.
I tested it quite thoroughly, and seems like there is no hopper duping happening with this fix.
In the process of testing this bug fix, I exposed another potentially serious and unrelated Minecraft bug.
Description in
MC-119971One other thing. I think it may be better to move the "removeAll" code to much earlier. There are things that happen between queueing chunks to unload and deleting the tile entities that may cause other chunks to get reloaded that were just queued. If that happens, we may have the analogous bugs to what is causing the hopper problem.
The way I have it now is to delete the tileentity deletion code from World.updateEntities and move it into its own function in World:
Then I insert a call to that RIGHT AFTER WorldServer.tick calls chunkProvider.unloadQueuedChunks, like this:
Confirmed in 1.12.1
I have not done a full analysis of the code between calling unloadQueuedChunks and doing removeAll. If the tile entities are removed right after being marked for removal as in my previous comment, then this is guaranteed safe with regard to tile entities.
If removeUnloadedTileEntities is called somewhere else, the thing we have to be careful about is with respect to other code in between unloadQueuedChunks and doing the removeAll. One thing in particular is ticking regular entities. Normally, they would not be ticked when unloaded chunks are nearby, but with multiple players, it's really easy to make chunk go directly from entity-processing to being unloaded, and this creates another opportunity for chunks to be unloaded and then reloaded on the same game tick. To be sure, the entity code deletes and ticks already in the right order. I'm just concerned about potential race conditions that haven't been fully ruled out. (Depending on what else is going on, we may want to delete regular entities earlier also!)
I was going to test these ideas, but
MC-2025got in my way. Although there have been fixes proposed earlier, I'm planning on doing a thorough analysis myself just to see if I can corroborate what others have said. Hopefully that will help add confidence to any community-proposed fixes. Either way, until 2025 is fixed, it's going to take some extra work for me to come up with a test case for regular entities and the interaction with loading and unloading, duping on chunk boundaries,MC-119971, etc.Thanks!
Marking as fixed for now. If something comes up with the entity code, let's create a new issue.
Thanks @devs and @mods for getting this fixed!
We can now rely on hopper clocks not breaking, as long as they don't straddle a chunk boundary!!
However, if hopper pairs are put on a chunk boundary, it's still really easy to get them to dupe items, so we may want to think a little bit about whether or not the bug-as-originally-reported is really fixed, given that it's still possible to make hoppers dupe items. One of the causes is fixed by applying the fix attached to
MC-79154(which is a HUGE improvement, so thank you), but another cause isn't. The fix for the chunk boundary case is attached toMC-119971.Long hopper lines for item delivery may still experience some problems, and pistons pushing blocks across chunk boundaries definitely will. Those are also fixed by the solution attached to
MC-119971. In fact, the fix attached toMC-119971is likely to also fix entity duplication and deletion that sometimes occurs at chunk boundaries. That fix is also being added to carpetmod so that community members can test it further.