Fix deadlock with ItemEntity + added ItemEntity#getMergeUpdateOption to mitigate CPU usage increase

This commit is contained in:
Felix Cravic 2020-06-01 00:51:31 +02:00
parent ea034701f8
commit 7e20278dd9
11 changed files with 80 additions and 48 deletions

View File

@ -141,8 +141,10 @@ public abstract class Entity implements Viewable, EventHandler, DataContainer {
/** /**
* Called each tick * Called each tick
*
* @param time the time of update in milliseconds
*/ */
public abstract void update(); public abstract void update(long time);
/** /**
* Called when a new instance is set * Called when a new instance is set
@ -449,7 +451,7 @@ public abstract class Entity implements Viewable, EventHandler, DataContainer {
handleVoid(); handleVoid();
// Call the abstract update method // Call the abstract update method
update(); update(time);
ticks++; ticks++;
callEvent(EntityTickEvent.class, tickEvent); // reuse tickEvent to avoid recreating it each tick callEvent(EntityTickEvent.class, tickEvent); // reuse tickEvent to avoid recreating it each tick

View File

@ -49,12 +49,11 @@ public abstract class EntityCreature extends LivingEntity {
} }
@Override @Override
public void update() { public void update(long time) {
super.update(); super.update(time);
// Path finding // Path finding
pathProgress(); pathProgress();
} }
/** /**

View File

@ -18,7 +18,7 @@ public class ExperienceOrb extends Entity {
} }
@Override @Override
public void update() { public void update(long time) {
// TODO slide toward nearest player // TODO slide toward nearest player
} }

View File

@ -6,13 +6,25 @@ import net.minestom.server.item.ItemStack;
import net.minestom.server.item.StackingRule; import net.minestom.server.item.StackingRule;
import net.minestom.server.network.packet.PacketWriter; import net.minestom.server.network.packet.PacketWriter;
import net.minestom.server.utils.Position; import net.minestom.server.utils.Position;
import net.minestom.server.utils.time.CooldownUtils;
import net.minestom.server.utils.time.TimeUnit; import net.minestom.server.utils.time.TimeUnit;
import net.minestom.server.utils.time.UpdateOption;
import java.util.Set; import java.util.Set;
import java.util.function.Consumer; import java.util.function.Consumer;
public class ItemEntity extends ObjectEntity { public class ItemEntity extends ObjectEntity {
/**
* Used to slow down the merge check delay
*/
private static UpdateOption mergeUpdateOption = new UpdateOption(10, TimeUnit.TICK);
/**
* The last time that this item has checked his neighbors for merge
*/
private long lastMergeCheck;
private ItemStack itemStack; private ItemStack itemStack;
private boolean pickable = true; private boolean pickable = true;
@ -28,9 +40,31 @@ public class ItemEntity extends ObjectEntity {
setBoundingBox(0.25f, 0.25f, 0.25f); setBoundingBox(0.25f, 0.25f, 0.25f);
} }
/**
* Get the update option for the merging feature
*
* @return the merge update option
*/
public static UpdateOption getMergeUpdateOption() {
return mergeUpdateOption;
}
/**
* Change the merge update option.
* Can be set to null to entirely remove the delay
*
* @param mergeUpdateOption the new merge update option
*/
public static void setMergeUpdateOption(UpdateOption mergeUpdateOption) {
ItemEntity.mergeUpdateOption = mergeUpdateOption;
}
@Override @Override
public void update() { public void update(long time) {
if (isMergeable() && isPickable()) { if (isMergeable() && isPickable() &&
(mergeUpdateOption != null && !CooldownUtils.hasCooldown(time, lastMergeCheck, mergeUpdateOption))) {
this.lastMergeCheck = time;
Chunk chunk = instance.getChunkAt(getPosition()); Chunk chunk = instance.getChunkAt(getPosition());
Set<Entity> entities = instance.getChunkEntities(chunk); Set<Entity> entities = instance.getChunkEntities(chunk);
for (Entity entity : entities) { for (Entity entity : entities) {
@ -48,30 +82,29 @@ public class ItemEntity extends ObjectEntity {
if (getDistance(itemEntity) > mergeRange) if (getDistance(itemEntity) > mergeRange)
continue; continue;
synchronized (this) { // Use the class as a monitor to prevent deadlock
synchronized (itemEntity) { // Shouldn't happen too often to be an issue
ItemStack itemStackEntity = itemEntity.getItemStack(); synchronized (ItemEntity.class) {
final ItemStack itemStackEntity = itemEntity.getItemStack();
StackingRule stackingRule = itemStack.getStackingRule(); final StackingRule stackingRule = itemStack.getStackingRule();
boolean canStack = stackingRule.canBeStacked(itemStack, itemStackEntity); final boolean canStack = stackingRule.canBeStacked(itemStack, itemStackEntity);
if (!canStack) if (!canStack)
continue; continue;
int totalAmount = stackingRule.getAmount(itemStack) + stackingRule.getAmount(itemStackEntity); final int totalAmount = stackingRule.getAmount(itemStack) + stackingRule.getAmount(itemStackEntity);
boolean canApply = stackingRule.canApply(itemStack, totalAmount); final boolean canApply = stackingRule.canApply(itemStack, totalAmount);
if (!canApply) if (!canApply)
continue; continue;
EntityItemMergeEvent entityItemMergeEvent = new EntityItemMergeEvent(this, itemEntity); EntityItemMergeEvent entityItemMergeEvent = new EntityItemMergeEvent(this, itemEntity);
callCancellableEvent(EntityItemMergeEvent.class, entityItemMergeEvent, () -> { callCancellableEvent(EntityItemMergeEvent.class, entityItemMergeEvent, () -> {
ItemStack result = stackingRule.apply(itemStack.clone(), totalAmount); ItemStack result = stackingRule.apply(itemStack.clone(), totalAmount);
setItemStack(result); setItemStack(result);
itemEntity.remove(); itemEntity.remove();
}); });
}
} }
} }

View File

@ -65,14 +65,14 @@ public abstract class LivingEntity extends Entity implements EquipmentHandler {
} }
@Override @Override
public void update() { public void update(long time) {
if (isOnFire()) { if (isOnFire()) {
if (System.currentTimeMillis() > fireExtinguishTime) { if (time > fireExtinguishTime) {
setOnFire(false); setOnFire(false);
} else { } else {
if (System.currentTimeMillis() - lastFireDamageTime > fireDamagePeriod) { if (time - lastFireDamageTime > fireDamagePeriod) {
damage(DamageType.ON_FIRE, 1.0f); damage(DamageType.ON_FIRE, 1.0f);
lastFireDamageTime = System.currentTimeMillis(); lastFireDamageTime = time;
} }
} }
} }
@ -92,6 +92,7 @@ public abstract class LivingEntity extends Entity implements EquipmentHandler {
ItemEntity itemEntity = (ItemEntity) entity; ItemEntity itemEntity = (ItemEntity) entity;
if (!itemEntity.isPickable()) if (!itemEntity.isPickable())
continue; continue;
BoundingBox itemBoundingBox = itemEntity.getBoundingBox(); BoundingBox itemBoundingBox = itemEntity.getBoundingBox();
if (livingBoundingBox.intersect(itemBoundingBox)) { if (livingBoundingBox.intersect(itemBoundingBox)) {
synchronized (itemEntity) { synchronized (itemEntity) {

View File

@ -19,7 +19,7 @@ public abstract class ObjectEntity extends Entity {
public abstract int getObjectData(); public abstract int getObjectData();
@Override @Override
public void update() { public void update(long time) {
} }

View File

@ -266,7 +266,7 @@ public class Player extends LivingEntity {
} }
@Override @Override
public void update() { public void update(long time) {
// Flush all pending packets // Flush all pending packets
playerConnection.flush(); playerConnection.flush();
@ -276,12 +276,12 @@ public class Player extends LivingEntity {
packet.process(this); packet.process(this);
} }
super.update(); // Super update (item pickup/fire management) super.update(time); // Super update (item pickup/fire management)
// Target block stage // Target block stage
if (targetCustomBlock != null) { if (targetCustomBlock != null) {
final byte animationCount = 10; final byte animationCount = 10;
long since = System.currentTimeMillis() - targetBlockTime; long since = time - targetBlockTime;
byte stage = (byte) (since / (blockBreakTime / animationCount)); byte stage = (byte) (since / (blockBreakTime / animationCount));
stage = MathUtils.setBetween(stage, (byte) -1, animationCount); stage = MathUtils.setBetween(stage, (byte) -1, animationCount);
if (stage != targetLastStage) { if (stage != targetLastStage) {
@ -317,7 +317,7 @@ public class Player extends LivingEntity {
// Eating animation // Eating animation
if (isEating()) { if (isEating()) {
if (System.currentTimeMillis() - startEatingTime >= eatingTime) { if (time - startEatingTime >= eatingTime) {
refreshEating(false); refreshEating(false);
triggerStatus((byte) 9); // Mark item use as finished triggerStatus((byte) 9); // Mark item use as finished

View File

@ -22,16 +22,6 @@ public class EntityBoat extends ObjectEntity {
return 0; return 0;
} }
@Override
public void update() {
}
@Override
public void spawn() {
}
@Override @Override
public Consumer<PacketWriter> getMetadataConsumer() { public Consumer<PacketWriter> getMetadataConsumer() {
return packet -> { return packet -> {

View File

@ -3,6 +3,7 @@ package net.minestom.server.event.player;
import net.minestom.server.entity.Player; import net.minestom.server.entity.Player;
import net.minestom.server.event.CancellableEvent; import net.minestom.server.event.CancellableEvent;
import net.minestom.server.item.ItemStack; import net.minestom.server.item.ItemStack;
import net.minestom.server.utils.item.ItemStackUtils;
/** /**
* Called as a result of {@link net.minestom.server.inventory.PlayerInventory#addItemStack(ItemStack)} * Called as a result of {@link net.minestom.server.inventory.PlayerInventory#addItemStack(ItemStack)}
@ -41,6 +42,6 @@ public class PlayerAddItemStackEvent extends CancellableEvent {
* @param itemStack the new item stack * @param itemStack the new item stack
*/ */
public void setItemStack(ItemStack itemStack) { public void setItemStack(ItemStack itemStack) {
this.itemStack = itemStack; this.itemStack = ItemStackUtils.notNull(itemStack);
} }
} }

View File

@ -3,9 +3,11 @@ package net.minestom.server.event.player;
import net.minestom.server.entity.Player; import net.minestom.server.entity.Player;
import net.minestom.server.event.CancellableEvent; import net.minestom.server.event.CancellableEvent;
import net.minestom.server.item.ItemStack; import net.minestom.server.item.ItemStack;
import net.minestom.server.utils.item.ItemStackUtils;
/** /**
* Called as a result of {@link net.minestom.server.inventory.PlayerInventory#setItemStack(int, ItemStack)} * Called as a result of {@link net.minestom.server.inventory.PlayerInventory#setItemStack(int, ItemStack)}
* and player click in his inventory
*/ */
public class PlayerSetItemStackEvent extends CancellableEvent { public class PlayerSetItemStackEvent extends CancellableEvent {
@ -61,7 +63,7 @@ public class PlayerSetItemStackEvent extends CancellableEvent {
* @param itemStack the new item stack * @param itemStack the new item stack
*/ */
public void setItemStack(ItemStack itemStack) { public void setItemStack(ItemStack itemStack) {
this.itemStack = itemStack; this.itemStack = ItemStackUtils.notNull(itemStack);
} }
} }

View File

@ -7,6 +7,10 @@ public class CooldownUtils {
return currentTime - lastUpdate < cooldownMs; return currentTime - lastUpdate < cooldownMs;
} }
public static boolean hasCooldown(long currentTime, long lastUpdate, UpdateOption updateOption) {
return hasCooldown(currentTime, lastUpdate, updateOption.getTimeUnit(), updateOption.getValue());
}
public static boolean hasCooldown(long lastUpdate, TimeUnit timeUnit, int cooldown) { public static boolean hasCooldown(long lastUpdate, TimeUnit timeUnit, int cooldown) {
return hasCooldown(System.currentTimeMillis(), lastUpdate, timeUnit, cooldown); return hasCooldown(System.currentTimeMillis(), lastUpdate, timeUnit, cooldown);
} }