fix(trading): prevent duplicate active trades

Guard RoomTradeManager.startTrade while holding the activeTrades lock so concurrent trade starts cannot register the same participant in multiple active trades before room status updates settle.

Add a contract test covering the lock-scoped participant guard and keep the existing trade safety tests green.
This commit is contained in:
simoleo89
2026-06-14 19:25:16 +02:00
parent c25cb2a9b6
commit 478c4c70b8
2 changed files with 48 additions and 1 deletions
@@ -19,8 +19,13 @@ public class RoomTradeManager {
* Starts a trade between two users.
*/
public void startTrade(Habbo userOne, Habbo userTwo) {
RoomTrade trade = new RoomTrade(userOne, userTwo, this.room);
RoomTrade trade;
synchronized (this.activeTrades) {
if (this.hasActiveTrade(userOne) || this.hasActiveTrade(userTwo)) {
return;
}
trade = new RoomTrade(userOne, userTwo, this.room);
this.activeTrades.add(trade);
}
@@ -58,4 +63,16 @@ public class RoomTradeManager {
public THashSet<RoomTrade> getActiveTrades() {
return this.activeTrades;
}
private boolean hasActiveTrade(Habbo user) {
for (RoomTrade trade : this.activeTrades) {
for (RoomTradeUser habbo : trade.getRoomTradeUsers()) {
if (habbo.getHabbo() == user) {
return true;
}
}
}
return false;
}
}
@@ -0,0 +1,30 @@
package com.eu.habbo.habbohotel.rooms;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.nio.file.Files;
import java.nio.file.Path;
import org.junit.jupiter.api.Test;
class RoomTradeManagerContractTest {
private static String roomTradeManagerSource() throws Exception {
return Files.readString(Path.of("src/main/java/com/eu/habbo/habbohotel/rooms/RoomTradeManager.java"));
}
@Test
void startTradeRejectsParticipantsAlreadyInActiveTradeInsideLock() throws Exception {
String source = roomTradeManagerSource();
int synchronizedBlock = source.indexOf("synchronized (this.activeTrades)");
int activeGuard = source.indexOf("hasActiveTrade(userOne) || this.hasActiveTrade(userTwo)");
int addTrade = source.indexOf("this.activeTrades.add(trade)");
assertTrue(synchronizedBlock > -1, "RoomTradeManager.startTrade must lock activeTrades before mutation");
assertTrue(activeGuard > synchronizedBlock,
"startTrade must check both participants for an existing active trade while holding the activeTrades lock");
assertTrue(activeGuard < addTrade,
"duplicate participant guard must run before a new RoomTrade is added");
assertTrue(source.contains("private boolean hasActiveTrade(Habbo user)"),
"active trade lookup should be reusable under the same activeTrades lock");
}
}