Bug with uShop 2.2.0 - Githubissues

itsgaryk commented 4 years ago

What steps will reproduce the issue?

One player enters /sell
Another player enter /sell

What was supposed to happen?

It should display the total value of items It should return the items if added to the sell box and the user cancels

What happened instead?

It shows value $0 for items of any quantity It does not return the items

What version of this plugin are you using?

2.2.0

What Spigot version are you using? Paste the output of /version below.

PaperMC build 130

What plugins do you have installed? Paste the output of /plugins below.

BackupOnEvent, ConsoleSpamFix, DeadChest, DiscordSRV, DropHeads, dynmap, Essentials, EssentialsChat, EssentialsSpawn, GriefPrevention, InstantBreak, LuckPerms, Pingas, PlugMan, ProtocolLib, SilkSpawners, SmoothSleep, uShop, Vault, WorldEdit

Are any errors related to this plugin in your console or logs? If so, paste below.

No visible errors

Any additional information that you would like to provide that may be relevant to the issue?

The bug can be reproduced after restarting the server. See the below image.

itsgaryk commented 4 years ago

I believe the same issue has been raised, however they have not mentioned items failing to return to the player https://github.com/mastercake10/uShop/issues/18

kotarobo commented 4 years ago

I'm seeing the same problem. I've also observed a deadlock and thread proliferation after that glitch occurred. The stack trace shows the following My server crashes when it grows by about 30,000.

`"Craft Scheduler Thread - 21152" #149360 prio=5 os_prio=0 tid=0x00007f2e730db970 nid=0xc2f7 waiting for monitor entry [0x00007f2c55ab7000] java.lang.Thread.State: BLOCKED (on object monitor) at xyz.spaceio.ushop.Main.lambda$0(Main.java:82)

waiting to lock <0x00000003d3e28d48> (a java.util.HashMap) at xyz.spaceio.ushop.Main$$Lambda$5569/1009543631.run(Unknown Source) at org.bukkit.craftbukkit.v1_16_R1.scheduler.CraftTask.run(CraftTask.java:99) at org.bukkit.craftbukkit.v1_16_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:54) at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at java.lang.Thread.run(Thread.java:748) "Craft Scheduler Thread - 276" #2676 prio=5 os_prio=0 tid=0x00007f32a40adec0 nid=0x3af4 runnable [0x00007f31649fa000] java.lang.Thread.State: RUNNABLE at java.lang.Throwable.fillInStackTrace(Native Method) at java.lang.Throwable.fillInStackTrace(Throwable.java:783)
locked <0x00000007003fd548> (a java.util.ConcurrentModificationException) at java.lang.Throwable.(Throwable.java:250) at java.lang.Exception.(Exception.java:54) at java.lang.RuntimeException.(RuntimeException.java:51) at java.util.ConcurrentModificationException.(ConcurrentModificationException.java:77) at java.util.HashMap$HashIterator.nextNode(HashMap.java:1442) at java.util.HashMap$KeyIterator.next(HashMap.java:1466) at xyz.spaceio.ushop.Main.lambda$0(Main.java:87)
locked <0x00000003d3e28d48> (a java.util.HashMap) at xyz.spaceio.ushop.Main$$Lambda$5569/1009543631.run(Unknown Source) at org.bukkit.craftbukkit.v1_16_R1.scheduler.CraftTask.run(CraftTask.java:99) at org.bukkit.craftbukkit.v1_16_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:54) at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at java.lang.Thread.run(Thread.java:748)`

MellnasMellicia commented 4 years ago

I believe the following code is the source of the problem? src/xyz/spaceio/ushop/Main.java(Line 298) this.getOpenShops().put(p, inv); This code changes the HashMap, but the Because it is not synchronized, a ConcurrentModificationException is thrown. In that case, the iterator needs to be regenerated, but since it was created outside of a while statement, it looks like an infinite loop of calling next(), throwing ConcrrentModificationException, continue, next().... Also, synchronized is not released from another thread, so it's a deadlocked state. I think this is also the reason why a lot of threads are created and not released.

mastercake10 commented 4 years ago

I think I've found the issue. The plugin was calling openShops.remove(p) while iterating the objects synchronized, which of course will throw an exception.

Removing the object from the iterator seems to solve the problem (it.remove()).

I'll file an alpha release, lets see if this will fix your issues.

2.2.1-alpha