Fix stale-message risk in async chat handling and shared-state bug in IslandChatCommand

src/main/java/world/bentobox/chat/commands/island/IslandChatCommand.java

@@ -2,8 +2,6 @@
 
 import java.util.List;
 
-import org.eclipse.jdt.annotation.Nullable;
-
 import world.bentobox.bentobox.api.commands.CompositeCommand;
 import world.bentobox.bentobox.api.user.User;
 import world.bentobox.bentobox.database.objects.Island;
@@ -14,8 +12,6 @@
  */
 public class IslandChatCommand extends CompositeCommand {
 
-    private @Nullable Island island;
-
     public IslandChatCommand(Chat addon, CompositeCommand parent, String label) {
         super(addon, parent, label);
     }
@@ -30,15 +26,23 @@ public void setup() {
 
     @Override
     public boolean canExecute(User user, String label, List<String> args) {
-
-        island = this.getIslands().getIslandAt(user.getLocation()).orElse(null);
-        return island != null;
+        // Command instances are shared across all players, so the resolved island must not be
+        // cached on an instance field here (e.g. player A's island could otherwise leak into
+        // player B's execute() call) - it is recomputed per-player in execute() instead.
+        return this.getIslands().getIslandAt(user.getLocation()).isPresent();
     }
 
     @Override
     public boolean execute(User user, String label, List<String> args) {
         Chat addon = this.getAddon();
 
+        // Resolve the island fresh for this specific invocation, since this command
+        // instance is shared by all players and must not rely on state set in canExecute().
+        Island island = this.getIslands().getIslandAt(user.getLocation()).orElse(null);
+        if (island == null) {
+            return false;
+        }
+
         // Send the message directly into island chat without the need of toggling it
         // if there is existence of more arguments
         if (!args.isEmpty()) {

src/test/java/world/bentobox/chat/listeners/ChatListenerTest.java

@@ -271,6 +271,68 @@ public void testOnChatIslandChatPlayerNotRegistered() {
         assertFalse(event.isCancelled());
     }
 
+    // -----------------------------------------------------------------------
+    // onChat asynchronous tests - regression tests for repeating message bug
+    // -----------------------------------------------------------------------
+
+    /**
+     * Stubs the scheduler so that {@code callSyncMethod} runs the given callable
+     * immediately (as if the next server tick had already happened) and returns
+     * an already completed future, mimicking the real Bukkit scheduler behaviour
+     * closely enough for the async {@link ChatListener#onChat(AsyncPlayerChatEvent)}
+     * code path to be exercised in a unit test.
+     */
+    private void stubSyncScheduler() {
+        when(sch.callSyncMethod(any(), any())).thenAnswer(invocation -> {
+            java.util.concurrent.Callable<?> callable = invocation.getArgument(1);
+            try {
+                return java.util.concurrent.CompletableFuture.completedFuture(callable.call());
+            } catch (Exception ex) {
+                throw new RuntimeException(ex);
+            }
+        });
+    }
+
+    @Test
+    public void testOnChatAsyncIslandChatUsesCurrentMessageNotStale() {
+        stubSyncScheduler();
+
+        ChatListener spyListener = org.mockito.Mockito.spy(listener);
+        spyListener.toggleIslandChat(island, player);
+        when(im.getIslandAt(any())).thenReturn(Optional.of(island));
+
+        // First async chat message
+        AsyncPlayerChatEvent event1 = new AsyncPlayerChatEvent(true, player, "aaaa", Collections.emptySet());
+        spyListener.onChat(event1);
+        assertTrue(event1.isCancelled());
+        verify(spyListener).islandChat(island, player, "aaaa");
+
+        // Second async chat message must use its own content, not the previous one
+        AsyncPlayerChatEvent event2 = new AsyncPlayerChatEvent(true, player, "test", Collections.emptySet());
+        spyListener.onChat(event2);
+        assertTrue(event2.isCancelled());
+        verify(spyListener).islandChat(island, player, "test");
+    }
+
+    @Test
+    public void testOnChatAsyncTeamChatUsesCurrentMessageNotStale() {
+        stubSyncScheduler();
+
+        ChatListener spyListener = org.mockito.Mockito.spy(listener);
+        spyListener.togglePlayerTeamChat(uuid);
+        when(im.inTeam(any(World.class), any(UUID.class))).thenReturn(true);
+
+        AsyncPlayerChatEvent event1 = new AsyncPlayerChatEvent(true, player, "aaaa", Collections.emptySet());
+        spyListener.onChat(event1);
+        assertTrue(event1.isCancelled());
+        verify(spyListener).teamChat(world, player, "aaaa");
+
+        AsyncPlayerChatEvent event2 = new AsyncPlayerChatEvent(true, player, "test", Collections.emptySet());
+        spyListener.onChat(event2);
+        assertTrue(event2.isCancelled());
+        verify(spyListener).teamChat(world, player, "test");
+    }
+
     // -----------------------------------------------------------------------
     // teamChat tests
     // -----------------------------------------------------------------------