Skip to content

fix: refresh permission snapshots so they stop expiring after 10 minutes#7

Merged
hbrombeer merged 1 commit into
mainfrom
fix/snapshot-eviction
Jul 13, 2026
Merged

fix: refresh permission snapshots so they stop expiring after 10 minutes#7
hbrombeer merged 1 commit into
mainfrom
fix/snapshot-eviction

Conversation

@hbrombeer

Copy link
Copy Markdown
Member

The bug

service-permissions stamps every snapshot with refreshAfter = now + 5min and expiresAt = now + 10min. SnapshotPermissions.hasPermission fails closed once the snapshot expires:

if (!snapshot.expiresAt.isAfter(now)) {
    return false
}

But a snapshot is put() only at login. There is no refresh path anywhere — no scheduler, no re-fetch, and RefreshOnlinePlayers on the service is a stub that logs "refresh accepted" and does nothing. InMemoryPermissionSnapshots.replaceAll was dead code.

Three consequences, one root cause:

  1. Every player silently loses all permissions 10 minutes after joining. An admin stops being an admin mid-session.
  2. A grant or revoke never reaches an online player — it needs a relog.
  3. Snapshots are never evicted, so the map grows for the lifetime of the pod. Minestom pods are long-lived and host many matches.

The fix

A sweep on both platforms, every PERMISSIONS_REFRESH_INTERVAL_SECONDS (default 60), that re-fetches snapshots for online players once refreshAfter has passed. No proto change — the contract already said all of this, nothing was listening.

Two things that are easy to get wrong, and are deliberate here:

It merges into the live map, it does not replace it. A login that lands mid-sweep writes a snapshot the sweep never sampled; a blind replaceAll would wipe it and leave that player with no snapshot — no permissions at all — until the next tick. The sweep does blocking gRPC calls, so that window is real. keepsSnapshotWrittenByALoginThatRacedTheSweep fails against the replace-based version.

It drops only offline AND expired entries. An offline player's unexpired snapshot is exactly what the login path reads as its service-outage fallback (cached.expiresAt.isAfter(now)), so evicting on disconnect — the obvious fix for the leak — would have broken it. Offline+expired is the only combination that is provably unusable.

A failed refresh keeps the existing snapshot and retries on the next tick; only the login path may ever deny a player.

Also bumps runtime-api 0.3.0 → 0.4.0 (the module calls activeModuleProviders; the SPI is unchanged across those versions).

Verification

./gradlew build green. 7 unit tests on the sweep: refresh-when-due, skip-when-fresh, keep-on-fetch-failure, keep-offline-unexpired, drop-offline-expired, fetch-when-absent, and the login race above.

The module calls GroundsServerContext.activeModuleProviders, added in
0.3.0. The GroundsModule/GroundsModuleProvider SPI is unchanged across
0.3.0..0.4.0, so this is a straight bump.
@hbrombeer
hbrombeer marked this pull request as ready for review July 13, 2026 18:14
@hbrombeer
hbrombeer merged commit 7ae5bd8 into main Jul 13, 2026
2 checks passed
@hbrombeer
hbrombeer deleted the fix/snapshot-eviction branch July 13, 2026 18:14
hbrombeer added a commit that referenced this pull request Jul 13, 2026
The sweep landed in #7 under a chore: subject, so release-please never cut a release and the fix was sitting on main unpublished.

Release-As: 0.4.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant