Skip to content

fix: validate region ownership of wanted item targets - #527

Open
PluginsKers wants to merge 1 commit into
PaperMC:ver/26.3.xfrom
PluginsKers:fix/wanted-item-region-26.3
Open

PluginsKers wants to merge 1 commit into
PaperMC:ver/26.3.xfrom
PluginsKers:fix/wanted-item-region-26.3

Conversation

@PluginsKers

Copy link
Copy Markdown

GoToWantedItem reads a cached item without checking whether the current region still owns it. For allays, EntityTargetEvent can also replace the target or begin transferring the original item before the behavior calls CraftItem.getHandle. That checked handle access throws for an unowned target; Level.guardEntityTick then discards the allay.

Check ownership of the remembered item before the predicate, distance, and world-border reads. Recheck the event's item target after cancellation and non-item handling, before getHandle. Bukkit.isOwnedByCurrentRegion uses CraftServer -> CraftEntity.getHandleRaw -> TickThread.isTickThreadFor, so the check does not itself invoke the checked handle accessor. Rejecting an unowned item returns false before creating EntityTracker instances or changing look/walk memories. Existing cancellation, null/non-item memory erasure, local replacement, pickup eligibility, and walk-interruption behavior remain unchanged.

The shared callers are AllayAi.initIdleActivity, PiglinAi.initAdmireItemActivity, and VillagerGoalPackages.getCorePackage. The production stack specifically identifies the post-event CraftItem.getHandle failure; it does not distinguish a stale original target from a plugin replacement. This change covers the consumer and event boundary rather than relying on a sensor-time ownership check.

Complete captured production call chain (26.2-7-14b7fee, entry to failure)
java.base/java.lang.Thread.run(Thread.java:1474)
 -> ca.spottedleaf.concurrentutil.util.LazyRunnable.run(LazyRunnable.java:25)
 -> ca.spottedleaf.concurrentutil.scheduler.StealingScheduledThreadPool$TickThreadRunner.run(StealingScheduledThreadPool.java:1150)
 -> ca.spottedleaf.concurrentutil.scheduler.StealingScheduledThreadPool$TickThreadRunner.doRun(StealingScheduledThreadPool.java:981)
 -> ca.spottedleaf.concurrentutil.scheduler.StealingScheduledThreadPool$TickThreadRunner.mainLoop(StealingScheduledThreadPool.java:1136)
 -> ca.spottedleaf.concurrentutil.scheduler.StealingScheduledThreadPool$TickThreadRunner.tick(StealingScheduledThreadPool.java:1086)
 -> io.papermc.paper.threadedregions.TickRegionScheduler$RegionScheduleHandle.runTick(TickRegionScheduler.java:481)
 -> io.papermc.paper.threadedregions.TickRegions$ConcreteRegionTickHandle.tickRegion(TickRegions.java:485)
 -> net.minecraft.server.dedicated.DedicatedServer.tickServer(DedicatedServer.java:406)
 -> net.minecraft.server.MinecraftServer.tickServer(MinecraftServer.java:1742)
 -> net.minecraft.server.MinecraftServer.tickChildren(MinecraftServer.java:1942)
 -> net.minecraft.server.level.ServerLevel.tick(ServerLevel.java:918)
 -> io.papermc.paper.threadedregions.RegionizedWorldData.forEachTickingEntity(RegionizedWorldData.java:615)
 -> net.minecraft.server.level.ServerLevel.lambda$tick$0(ServerLevel.java:937)
 -> net.minecraft.world.level.Level.guardEntityTick(Level.java:1573)
 -> net.minecraft.server.level.ServerLevel.tickNonPassenger(ServerLevel.java:1496)
 -> net.minecraft.world.entity.animal.allay.Allay.tick(Allay.java:238)
 -> net.minecraft.world.entity.Mob.tick(Mob.java:422)
 -> net.minecraft.world.entity.LivingEntity.tick(LivingEntity.java:3460)
 -> net.minecraft.world.entity.animal.allay.Allay.aiStep(Allay.java:223)
 -> net.minecraft.world.entity.Mob.aiStep(Mob.java:544)
 -> net.minecraft.world.entity.LivingEntity.aiStep(LivingEntity.java:3754)
 -> net.minecraft.world.entity.Mob.serverAiStep(Mob.java:863)
 -> net.minecraft.world.entity.animal.allay.Allay.customServerAiStep(Allay.java:213)
 -> net.minecraft.world.entity.ai.Brain.tick(Brain.java:388)
 -> net.minecraft.world.entity.ai.Brain.startEachNonRunningBehavior(Brain.java:427)
 -> net.minecraft.world.entity.ai.behavior.OneShot.tryStart(OneShot.java:17)
 -> net.minecraft.world.entity.ai.behavior.declarative.BehaviorBuilder$1.trigger(BehaviorBuilder.java:46)
 -> net.minecraft.world.entity.ai.behavior.GoToWantedItem.lambda$create$3(GoToWantedItem.java:52)
 -> org.bukkit.craftbukkit.entity.CraftItem.getHandle(CraftItem.java:24)
 -> ca.spottedleaf.moonrise.common.util.TickThread.ensureTickThread(TickThread.java:98)

Original exception stack. These method names and line numbers belong to the captured 26.2 build.

  1. Follow the standalone build and server setup, using baseline 597e41371802648e0455fc1571192ee1c83fca2b and this PR's commit for the fixed variant. Use the same probe JAR in fresh, loopback-only server directories and run java -Xms1G -Xmx2G -Duser.language=en -Dfolia.probe=item -jar folia.jar --nogui.
  2. The fixture keeps chunks (0, 0) and (512, 0) active in separate regions. On their owner regions, create an allay at (3.5, 65.5, 3.5) holding stone, a local stone item, and a remote stone item. Disable autonomous AI, enable item pickup, erase pickup cooldown, and seed wanted/look/walk memories with known references.
  3. Register an EntityTargetEvent listener for that allay and CLOSEST_ENTITY. Replace event.target with the remote item's Bukkit wrapper. Invoke GoToWantedItem.create(body -> true, 1.75f, true, 32).tryStart inside level.guardEntityTick on the allay's region. The unchanged server throws at CraftItem.getHandle and removes the allay. With the guards, it returns false, keeps the allay, and preserves the seeded memories.
  4. Repeat with a foreign-dimension event replacement and a listener that starts teleportAsync on the original item while leaving event.target unchanged. The latter case asserts that the item has relinquished region ownership before the listener returns.
  5. To isolate the initial guard, seed a live item from another dimension at nearby numeric coordinates for an allay, piglin, and villager. The fixed behavior rejects it before range reads, events, or tracker creation. A distant same-world item already rejected by the range gate is only a control. The fixture also covers local targets, an existing villager walk, cancellation, null/non-item event results, and local replacements.

The production failure identifies the post-event checked accessor. The fixture establishes each ownership precondition explicitly; it does not claim to reconstruct the production listener or the original source of stale memory.

The current 26.3 failure stack, baseline log, and fixed log are pinned alongside the fixture. The recorded fixed runtime contains the three independent guards; this PR contains only the item-target change. The reproductions invoke real behaviors on owner threads with autonomous AI disabled; separate single-fix runtime JARs and complete autonomous gameplay runs were not recorded. Read assertion markers in the logs, since the fixture shuts down normally even after an assertion fails.

AI tools assisted the investigation, implementation, reproduction, and this description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant