Shared player and cosmetic collections are mutated across threads without synchronisation #6

New Issue

2026-09-25T11:43:41Z

selimaj-dev commented

2026-09-25 11:43:41 +00:00

Problem

Several collections are written from background threads and read on the render thread, with no synchronisation:

Collection	Written from	Read from
`SaturnPlayer.PLAYERS` (`HashMap`)	render thread (`get` → `put(uuid, null)`), auth thread (`SaturnPlayer.set`), the notification thread (`player` notification → `SaturnPlayer.set`)	render thread, `getExternalUUIDAsString()`
`Cloaks.availableCloaks`, `Hats.availableHats` (`ArrayList`)	auth thread (`authenticate`), purchase callbacks (`buyCloak` / `buyHat` `whenComplete`)	store and cosmetics screens on the render thread

getExternalUUIDAsString() streams over PLAYERS.keySet() while other threads insert. That's a textbook ConcurrentModificationException, and a HashMap resize during a concurrent write can also corrupt the map. Iterating the cosmetic lists in the UI while a purchase completes has the same problem.

session-java runs notification handlers on a CompletableFuture.runAsync pool thread, so notifications always arrive off the main thread.

Fix

Either:

apply every state change on the client thread with Providers.saturn.getClient().executeOnThread(...), as the player worker already does in one place, or
switch to ConcurrentHashMap and CopyOnWriteArrayList. ConcurrentHashMap doesn't allow null values, so this goes together with the "not a Saturn user" marker from the get_player issue.

Moving writes onto the client thread is the more robust option, since the render code reads these without locks.

## Problem Several collections are written from background threads and read on the render thread, with no synchronisation: | Collection | Written from | Read from | | --- | --- | --- | | `SaturnPlayer.PLAYERS` (`HashMap`) | render thread (`get` → `put(uuid, null)`), auth thread (`SaturnPlayer.set`), the notification thread (`player` notification → `SaturnPlayer.set`) | render thread, `getExternalUUIDAsString()` | | `Cloaks.availableCloaks`, `Hats.availableHats` (`ArrayList`) | auth thread (`authenticate`), purchase callbacks (`buyCloak` / `buyHat` `whenComplete`) | store and cosmetics screens on the render thread | `getExternalUUIDAsString()` streams over `PLAYERS.keySet()` while other threads insert. That's a textbook `ConcurrentModificationException`, and a `HashMap` resize during a concurrent write can also corrupt the map. Iterating the cosmetic lists in the UI while a purchase completes has the same problem. `session-java` runs notification handlers on a `CompletableFuture.runAsync` pool thread, so notifications always arrive off the main thread. ## Fix Either: - apply every state change on the client thread with `Providers.saturn.getClient().executeOnThread(...)`, as the player worker already does in one place, **or** - switch to `ConcurrentHashMap` and `CopyOnWriteArrayList`. `ConcurrentHashMap` doesn't allow `null` values, so this goes together with the "not a Saturn user" marker from the `get_player` issue. Moving writes onto the client thread is the more robust option, since the render code reads these without locks.

selimaj-dev referenced this issue from a commit

2026-09-25 12:00:30 +00:00

Stop per-frame player lookups and make shared state thread-safe

selimaj-dev referenced this issue from saturnclientmc/common

2026-09-25 12:00:31 +00:00

Stop per-frame player lookups and make shared state thread-safe #1

selimaj-dev referenced this issue from a commit

2026-09-25 12:00:40 +00:00

Keep server tab names and team formatting in the tab list

selimaj-dev referenced this issue

2026-09-25 12:00:53 +00:00

Keep server tab names and team formatting; fix player lookups #9