From ccea10653377d1805073ba7c705b501b2094e548 Mon Sep 17 00:00:00 2001 From: TomyLobo Date: Sun, 11 Jun 2023 00:07:43 +0200 Subject: [PATCH 1/2] Synchronize YamlRegionFile#saveAll This prevents multiple invocations of the method from intersecting, which could potentially cause the .tmp file to be overwritten before being renamed, thus corrupting the region storage --- .../protection/managers/storage/file/YamlRegionFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/storage/file/YamlRegionFile.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/storage/file/YamlRegionFile.java index 08e125aef..9c0dd8f69 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/storage/file/YamlRegionFile.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/storage/file/YamlRegionFile.java @@ -191,7 +191,7 @@ public Set loadAll(FlagRegistry flagRegistry) throws StorageExc } @Override - public void saveAll(Set regions) throws StorageException { + public synchronized void saveAll(Set regions) throws StorageException { checkNotNull(regions); File tempFile = new File(file.getParentFile(), file.getName() + ".tmp"); From 4d06481d7bb0a9b768b571cb054be9581285fa32 Mon Sep 17 00:00:00 2001 From: TomyLobo Date: Mon, 12 Jun 2023 03:16:14 +0200 Subject: [PATCH 2/2] Synchronize on the class instead of the instance I didn't realize it before, but it looks like there is at least one instance of YamlRegionFile per world, so synchronizing on the instance wouldn't necessarily help. --- .../managers/storage/file/YamlRegionFile.java | 106 +++++++++--------- 1 file changed, 54 insertions(+), 52 deletions(-) diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/storage/file/YamlRegionFile.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/storage/file/YamlRegionFile.java index 9c0dd8f69..0d36e4dcf 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/storage/file/YamlRegionFile.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/storage/file/YamlRegionFile.java @@ -191,66 +191,68 @@ public Set loadAll(FlagRegistry flagRegistry) throws StorageExc } @Override - public synchronized void saveAll(Set regions) throws StorageException { + public void saveAll(Set regions) throws StorageException { checkNotNull(regions); - File tempFile = new File(file.getParentFile(), file.getName() + ".tmp"); - YAMLProcessor config = createYamlProcessor(tempFile); - - config.clear(); - - YAMLNode regionsNode = config.addNode("regions"); - Map map = regionsNode.getMap(); - - for (ProtectedRegion region : regions) { - Map nodeMap = new HashMap<>(); - map.put(region.getId(), nodeMap); - YAMLNode node = new YAMLNode(nodeMap, false); - - if (region instanceof ProtectedCuboidRegion) { - ProtectedCuboidRegion cuboid = (ProtectedCuboidRegion) region; - node.setProperty("type", "cuboid"); - node.setProperty("min", cuboid.getMinimumPoint()); - node.setProperty("max", cuboid.getMaximumPoint()); - } else if (region instanceof ProtectedPolygonalRegion) { - ProtectedPolygonalRegion poly = (ProtectedPolygonalRegion) region; - node.setProperty("type", "poly2d"); - node.setProperty("min-y", poly.getMinimumPoint().y()); - node.setProperty("max-y", poly.getMaximumPoint().y()); - - List> points = new ArrayList<>(); - for (BlockVector2 point : poly.getPoints()) { - Map data = new HashMap<>(); - data.put("x", point.x()); - data.put("z", point.z()); - points.add(data); + synchronized (YamlRegionFile.class) { + File tempFile = new File(file.getParentFile(), file.getName() + ".tmp"); + YAMLProcessor config = createYamlProcessor(tempFile); + + config.clear(); + + YAMLNode regionsNode = config.addNode("regions"); + Map map = regionsNode.getMap(); + + for (ProtectedRegion region : regions) { + Map nodeMap = new HashMap<>(); + map.put(region.getId(), nodeMap); + YAMLNode node = new YAMLNode(nodeMap, false); + + if (region instanceof ProtectedCuboidRegion) { + ProtectedCuboidRegion cuboid = (ProtectedCuboidRegion) region; + node.setProperty("type", "cuboid"); + node.setProperty("min", cuboid.getMinimumPoint()); + node.setProperty("max", cuboid.getMaximumPoint()); + } else if (region instanceof ProtectedPolygonalRegion) { + ProtectedPolygonalRegion poly = (ProtectedPolygonalRegion) region; + node.setProperty("type", "poly2d"); + node.setProperty("min-y", poly.getMinimumPoint().y()); + node.setProperty("max-y", poly.getMaximumPoint().y()); + + List> points = new ArrayList<>(); + for (BlockVector2 point : poly.getPoints()) { + Map data = new HashMap<>(); + data.put("x", point.x()); + data.put("z", point.z()); + points.add(data); + } + + node.setProperty("points", points); + } else if (region instanceof GlobalProtectedRegion) { + node.setProperty("type", "global"); + } else { + node.setProperty("type", region.getClass().getCanonicalName()); } - node.setProperty("points", points); - } else if (region instanceof GlobalProtectedRegion) { - node.setProperty("type", "global"); - } else { - node.setProperty("type", region.getClass().getCanonicalName()); - } - - node.setProperty("priority", region.getPriority()); - node.setProperty("flags", getFlagData(region)); - node.setProperty("owners", getDomainData(region.getOwners())); - node.setProperty("members", getDomainData(region.getMembers())); + node.setProperty("priority", region.getPriority()); + node.setProperty("flags", getFlagData(region)); + node.setProperty("owners", getDomainData(region.getOwners())); + node.setProperty("members", getDomainData(region.getMembers())); - ProtectedRegion parent = region.getParent(); - if (parent != null) { - node.setProperty("parent", parent.getId()); + ProtectedRegion parent = region.getParent(); + if (parent != null) { + node.setProperty("parent", parent.getId()); + } } - } - config.setHeader(FILE_HEADER); - config.save(); + config.setHeader(FILE_HEADER); + config.save(); - //noinspection ResultOfMethodCallIgnored - file.delete(); - if (!tempFile.renameTo(file)) { - throw new StorageException("Failed to rename temporary regions file to " + file.getAbsolutePath()); + //noinspection ResultOfMethodCallIgnored + file.delete(); + if (!tempFile.renameTo(file)) { + throw new StorageException("Failed to rename temporary regions file to " + file.getAbsolutePath()); + } } }