Skip to content

Make /co reload swap world configs and leave block groups alone - #1015

Merged
Intelli merged 4 commits into
PlayPro:masterfrom
tricrotism:for-upstream/reload-world-config-swap
Oct 2, 2026
Merged

Intelli merged 4 commits into
PlayPro:masterfrom
tricrotism:for-upstream/reload-world-config-swap

Conversation

@tricrotism

Copy link
Copy Markdown
Contributor

Summary

Per-world configs live in a plain HashMap that listeners on many threads read, and that Config.getConfig also writes to. /co reload clears it before refilling it. A listener that looks up its world in that window can overwrite the world's own config with the global one until the next reload. Reload also rebuilds the shared BlockGroup sets in place while listeners read them. This change makes the map concurrent and read-only for listeners, swaps world configs in without a gap, and builds the block groups only at startup.

The problem

  • Config.getConfig (config/Config.java:330-337) is called by nearly every listener, on region threads on Folia and on async threads everywhere. On a miss it writes into CONFIG_BY_WORLD_NAME:
    Config ret = CONFIG_BY_WORLD_NAME.get(worldName);
    if (ret == null) {
        ret = CONFIG_BY_WORLD_NAME.getOrDefault(worldName, GLOBAL);
        CONFIG_BY_WORLD_NAME.put(worldName, ret);
    }
    CONFIG_BY_WORLD_NAME is a plain HashMap, so this is an unsynchronized put from several threads at once.
  • /co reload runs CONFIG_BY_WORLD_NAME.clear() (Config.java:491) and then puts each world config back. Between the clear and the put, a listener for a world that has its own config (plugins/CoreProtect/world_nether.yml) misses and puts GLOBAL for that world. If that put lands after the reload has put the world's real config, the world keeps using the global config until the next /co reload. An admin who disabled, for example, item-transactions in one world sees it logged again after a reload, with nothing in the console to say why.
  • ConfigHandler.performInitialization re-runs BukkitAdapter.loadAdapter(), SpigotAdapter.loadAdapter(), PaperAdapter.loadAdapter() and BlockGroup.initialize() on every /co reload (config/ConfigHandler.java:916-919). The adapters replace some public BlockGroup sets with new HashSets and add to others (Bukkit_v1_19.java:87, Bukkit_v1_20.java:75), and initialize() adds to them again, while listeners on other threads iterate and test those same sets without a lock.

The fix

  • CONFIG_BY_WORLD_NAME is a ConcurrentHashMap.
  • getConfig only reads: CONFIG_BY_WORLD_NAME.getOrDefault(worldName, GLOBAL). A world without its own config gets GLOBAL as before, without writing it into the map.
  • Reload builds the new world configs into a local map, then putAlls them and removes the worlds whose config file is gone. A configured world reads either its old config or its new one, never the global one in between.
  • The adapters and BlockGroup.initialize() run only when performInitialization(true) is called at startup. They depend only on the server version (ConfigHandler.SERVER_VERSION, isPaper, isSpigot), which cannot change during /co reload. Their config reads (HOVER_EVENTS, MYSQL) happen at call time, so a reload still takes effect for them.

Behaviour change

None once a reload completes. During a reload, each world keeps its previous config until its new one is in place.

Risk

  • performInitialization(false) is only called from ReloadCommand, so plugin enable still builds everything.
  • GLOBAL itself is still reloaded in place, as before. That is a separate, narrower issue, and I have not touched it here.

Testing

Build: mvn package passes.
Reload check, a small plugin (ReloadCheck) on Paper 26.2 and Folia 1.21.11 with SQLite: dispense and explode, write a world config with item-transactions: false, /co reload, dispense and explode again, delete the world config, /co reload, dispense again. Upstream and this branch both logged 1 container row, then 0 with the world config, then 1 after it was removed, with the explosions logging the same 2 block rows each time and no errors. That shows reload still applies and removes world configs and that block groups still classify the explosion after a reload. It does not provoke the reload race itself.

Row parity: the 47-step scenario on Paper 26.2 with SQLite matches upstream apart from the random plant that bone meal grows.

Suggested test: add plugins/CoreProtect/<world>.yml with item-transactions: false, run /co reload, fire a dispenser and check no container row is logged. Remove the file, /co reload, fire again and check the row is logged. Repeat the reload many times while a dispenser clock runs, and confirm the world config is never lost.

The per-world config map was a plain HashMap that getConfig wrote into on a miss, and reload cleared it before refilling, so a listener hitting that window could leave a world on the global config until the next reload. The map is now concurrent and read-only for listeners, and reload swaps new configs in without a gap. Reload also stops rebuilding the version adapters and block groups, which only depend on the server version, while listeners read them.
@netlify

netlify Bot commented Sep 23, 2026

Copy link
Copy Markdown

❌ Deploy Preview for coreprotect failed. Why did it fail? →

Name Link
🔨 Latest commit 0500da4
🔍 Latest deploy log https://app.netlify.com/projects/coreprotect/deploys/6ab3eea9d72f8500081deff7

@Intelli

Intelli commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Thanks -- automated review is requesting the following changes:

  • Please reconsider making all adapter/block-group initialization startup-only. These groups also depend on datapack tags: Bukkit_v1_20.initializeTaggedBlocks() reads buttons, pressure plates, flowers, saplings, etc., and BlockGroup.initialize() reads additional tags. After /minecraft:reload, /co reload currently refreshes those values; this change would leave them stale until restart.
  • Please preserve tag-derived group refresh through safely published replacement sets, or keep this PR focused on the config-map fix and handle block-group concurrency separately. The claim that block groups depend only on server version should also be corrected.

Several block groups are filled from datapack tags (buttons, pressure
plates, doors, saplings, flowers, carpets, logs and more), so they are
not fixed by the server version. Running the adapters and
BlockGroup.initialize() only at startup would leave those groups stale
after a datapack reload until the server restarts. Restore the previous
behaviour and keep this change to the world config map.
@Intelli
Intelli merged commit 18643de into PlayPro:master Oct 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants