Skip to content

Commit 7edbce9

Browse files
authored
GH-1090 Fix rare race condition in SilentContainer. (#1090)
* Fix rare race condition in SilentContainer. * Follow gemini review.
1 parent 4bf8834 commit 7edbce9

File tree

1 file changed

+76
-39
lines changed

1 file changed

+76
-39
lines changed

eternalcore-core/src/main/java/com/eternalcode/core/feature/vanish/controller/OpenSilentController.java

Lines changed: 76 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@
66
import com.eternalcode.core.injector.annotations.Inject;
77
import com.eternalcode.core.injector.annotations.component.Controller;
88
import com.eternalcode.core.notice.NoticeService;
9+
import com.github.benmanes.caffeine.cache.Cache;
10+
import com.github.benmanes.caffeine.cache.Caffeine;
11+
import com.github.benmanes.caffeine.cache.RemovalCause;
12+
import com.github.benmanes.caffeine.cache.RemovalListener;
913
import java.time.Duration;
10-
import java.util.HashMap;
11-
import java.util.Map;
1214
import java.util.UUID;
15+
import java.util.concurrent.TimeUnit;
16+
import org.bukkit.Bukkit;
1317
import org.bukkit.GameMode;
1418
import org.bukkit.Material;
1519
import org.bukkit.Tag;
@@ -27,12 +31,15 @@
2731
import org.bukkit.event.player.PlayerTeleportEvent.TeleportCause;
2832
import org.bukkit.inventory.CraftingInventory;
2933
import org.bukkit.util.Vector;
34+
import org.jetbrains.annotations.NotNull;
3035

3136
/**
3237
* Controller allowing vanished players to silently access containers
3338
* without triggering animations/sounds for other players.
3439
* Temporarily switches player to Spectator mode for 1 tick.
3540
* <p>
41+
* Uses Caffeine cache for automatic cleanup and state management.
42+
* <p>
3643
* Implementation based on SayanVanish plugin approach.
3744
* Original idea: <a href="https://github.com/Syrent/SayanVanish/blob/master/sayanvanish-bukkit/src/main/kotlin/org/sayandev/sayanvanish/bukkit/feature/features/FeatureSilentContainer.kt">...</a>
3845
*/
@@ -41,24 +48,31 @@ class OpenSilentController implements Listener {
4148

4249
private static final Vector ZERO_VELOCITY = new Vector(0.0, 0.0, 0.0);
4350
private static final int TICK_DURATION_MILLIS = 50;
51+
private static final int CACHE_EXPIRE_SECONDS = 5;
4452

4553
private final NoticeService noticeService;
4654
private final VanishService vanishService;
4755
private final VanishSettings config;
4856
private final Scheduler scheduler;
4957

50-
private final Map<UUID, ContainerWrapper> containerCache = new HashMap<>();
58+
private final Cache<UUID, ContainerWrapper> containerCache;
5159

5260
@Inject
5361
OpenSilentController(
5462
NoticeService noticeService,
5563
VanishService vanishService,
5664
VanishSettings config,
57-
Scheduler scheduler) {
65+
Scheduler scheduler
66+
) {
5867
this.noticeService = noticeService;
5968
this.vanishService = vanishService;
6069
this.config = config;
6170
this.scheduler = scheduler;
71+
72+
this.containerCache = Caffeine.newBuilder()
73+
.expireAfterWrite(CACHE_EXPIRE_SECONDS, TimeUnit.SECONDS)
74+
.removalListener(new PlayerRestoreListener(scheduler))
75+
.build();
6276
}
6377

6478
@EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true)
@@ -77,7 +91,8 @@ void handlePlayerInteract(PlayerInteractEvent event) {
7791
event.setCancelled(true);
7892
this.noticeService.player(
7993
player.getUniqueId(),
80-
message -> message.vanish().cantOpenInventoryWhileVanished());
94+
message -> message.vanish().cantOpenInventoryWhileVanished()
95+
);
8196
return;
8297
}
8398

@@ -98,15 +113,9 @@ void handlePlayerInteract(PlayerInteractEvent event) {
98113
return;
99114
}
100115

101-
ContainerWrapper playerData =
102-
new ContainerWrapper(player.getGameMode(), player.getAllowFlight(), player.isFlying());
103-
this.containerCache.put(player.getUniqueId(), playerData);
104-
105-
this.switchToSpectator(player);
106-
this.restoreAfterTick(player);
116+
this.switchForOneTick(player);
107117
}
108118

109-
// Blocks unwanted teleports while in temporary spectator mode
110119
@EventHandler(priority = EventPriority.HIGHEST)
111120
private void handlePlayerTeleport(PlayerTeleportEvent event) {
112121
Player player = event.getPlayer();
@@ -116,27 +125,24 @@ private void handlePlayerTeleport(PlayerTeleportEvent event) {
116125
return;
117126
}
118127

119-
if (!this.containerCache.containsKey(player.getUniqueId())
120-
&& cause != PlayerTeleportEvent.TeleportCause.SPECTATE) {
121-
return;
128+
if (this.containerCache.getIfPresent(player.getUniqueId()) != null) {
129+
event.setCancelled(true);
122130
}
123-
124-
event.setCancelled(true);
125131
}
126132

127133
@EventHandler(priority = EventPriority.HIGHEST)
128134
private void handlePlayerQuit(PlayerQuitEvent event) {
129135
Player player = event.getPlayer();
130-
ContainerWrapper data = this.containerCache.get(player.getUniqueId());
136+
UUID playerId = player.getUniqueId();
131137

138+
ContainerWrapper data = this.containerCache.getIfPresent(playerId);
132139
if (data != null) {
140+
this.containerCache.invalidate(playerId);
133141
data.apply(player);
134142
}
135-
136-
this.containerCache.remove(player.getUniqueId());
137143
}
138144

139-
@EventHandler
145+
@EventHandler(priority = EventPriority.MONITOR)
140146
private void handleInventoryClose(InventoryCloseEvent event) {
141147
if (!(event.getPlayer() instanceof Player player)) {
142148
return;
@@ -150,29 +156,42 @@ private void handleInventoryClose(InventoryCloseEvent event) {
150156
return;
151157
}
152158

153-
ContainerWrapper playerData =
154-
new ContainerWrapper(player.getGameMode(), player.getAllowFlight(), player.isFlying());
155-
this.containerCache.put(player.getUniqueId(), playerData);
156-
157-
this.switchToSpectator(player);
158-
this.restoreAfterTick(player);
159+
if (this.containerCache.getIfPresent(player.getUniqueId()) == null) {
160+
this.switchForOneTick(player);
161+
}
159162
}
160163

161-
private void switchToSpectator(Player player) {
164+
private void switchForOneTick(Player player) {
165+
UUID playerId = player.getUniqueId();
166+
167+
if (this.containerCache.getIfPresent(playerId) != null) {
168+
return;
169+
}
170+
171+
ContainerWrapper playerData = new ContainerWrapper(
172+
player.getGameMode(),
173+
player.getAllowFlight(),
174+
player.isFlying()
175+
);
176+
this.containerCache.put(playerId, playerData);
177+
162178
player.setAllowFlight(true);
163179
player.setFlying(true);
164180
player.setVelocity(ZERO_VELOCITY);
165181
player.setGameMode(GameMode.SPECTATOR);
182+
this.scheduler.runLater(() -> this.restorePlayer(playerId), Duration.ofMillis(TICK_DURATION_MILLIS));
166183
}
167184

168-
private void restoreAfterTick(Player player) {
169-
this.scheduler.runLater(
170-
() -> {
171-
ContainerWrapper data = this.containerCache.remove(player.getUniqueId());
172-
if (data != null) {
173-
data.apply(player);
174-
}
175-
}, Duration.ofMillis(TICK_DURATION_MILLIS));
185+
private void restorePlayer(UUID playerId) {
186+
ContainerWrapper data = this.containerCache.getIfPresent(playerId);
187+
if (data != null) {
188+
this.containerCache.invalidate(playerId);
189+
190+
Player player = Bukkit.getPlayer(playerId);
191+
if (player != null && player.isOnline()) {
192+
data.apply(player);
193+
}
194+
}
176195
}
177196

178197
private boolean isContainerType(Material type) {
@@ -182,11 +201,29 @@ private boolean isContainerType(Material type) {
182201
};
183202
}
184203

204+
private record PlayerRestoreListener(Scheduler scheduler) implements RemovalListener<UUID, ContainerWrapper> {
205+
206+
@Override
207+
public void onRemoval(UUID playerId, ContainerWrapper data, @NotNull RemovalCause cause) {
208+
if (cause == RemovalCause.EXPIRED && data != null) {
209+
210+
this.scheduler.run(() -> {
211+
Player player = Bukkit.getPlayer(playerId);
212+
if (player != null && player.isOnline()) {
213+
data.apply(player);
214+
}
215+
});
216+
}
217+
}
218+
}
219+
185220
private record ContainerWrapper(GameMode gameMode, boolean allowFlight, boolean isFlying) {
186221
public void apply(Player player) {
187-
player.setGameMode(gameMode);
188-
player.setAllowFlight(allowFlight);
189-
player.setFlying(isFlying);
222+
if (player.isOnline()) {
223+
player.setGameMode(gameMode);
224+
player.setAllowFlight(allowFlight);
225+
player.setFlying(isFlying);
226+
}
190227
}
191228
}
192229
}

0 commit comments

Comments
 (0)