Skip to content

Commit 9a4cfca

Browse files
TechcableTechcable
committed
Use dependency injection instead of singleton access to the plugin
Fix incorrect double checked locking on spawnshield config. cachedRegionsToBlock wasn't volatile, and so not all threads may have seen the correct value.
1 parent 703a3cc commit 9a4cfca

File tree

8 files changed

+63
-67
lines changed

8 files changed

+63
-67
lines changed

‎base/src/main/java/net/techcable/spawnshield/SpawnShield.java‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,10 @@ public class SpawnShield extends TechPlugin<SpawnShieldPlayer> {
6464

6565
@Override
6666
protected void startup() {
67-
getInstance().getLogger().info("Loading SpawnShield by Techcable");
67+
getLogger().info("Loading SpawnShield by Techcable");
6868
if (getCombatTagPlugins().isEmpty()) {
69-
getInstance().getLogger().severe("No Combat Tagging Plugin Installed");
70-
getInstance().getLogger().severe("Shutting down");
69+
getLogger().severe("No Combat Tagging Plugin Installed");
70+
getLogger().severe("Shutting down");
7171
setEnabled(false);
7272
return;
7373
} else {
@@ -109,7 +109,7 @@ public int getValue() {
109109
});
110110
metrics.start();
111111
} catch (IOException e) {
112-
getInstance().getLogger().warning("Unable to run metrics");
112+
getLogger().warning("Unable to run metrics");
113113
}
114114
if (getSettings().isDebug()) {
115115
getLogger().setLevel(Level.FINE);
@@ -119,18 +119,18 @@ public int getValue() {
119119
if (Bukkit.getPluginManager().isPluginEnabled("WorldGuard")) {
120120
String version = Bukkit.getPluginManager().getPlugin("WorldGuard").getDescription().getVersion();
121121
if (version.startsWith("6")) {
122-
getInstance().getLogger().info("Worldguard 6 Detected, Activating support");
122+
getLogger().info("Worldguard 6 Detected, Activating support");
123123
WorldGuard6Plugin hook = new WorldGuard6Plugin();
124124
getSettings().addProtectionPlugin(hook);
125125
numPluginsAdded++;
126126
}
127127
}
128128
if (numPluginsAdded == 0) {
129-
getInstance().getLogger().severe("No supported protection plugin found, shutting down");
129+
getLogger().severe("No supported protection plugin found, shutting down");
130130
setEnabled(false);
131131
return;
132132
}
133-
getCommand("spawnshield").setExecutor(new SpawnShieldExecutor());
133+
getCommand("spawnshield").setExecutor(new SpawnShieldExecutor(this));
134134
switch (settings.getMode()) {
135135
case FORCEFIELD :
136136
this.forceFieldListener = new ForceFieldListener(this);
@@ -145,7 +145,7 @@ public int getValue() {
145145
knockbackTask.runTaskTimer(this, 5, 5); //Every 1/4 of a tick
146146
break;
147147
default :
148-
getInstance().getLogger().severe("[SpawnShield] Unknown Plugin Mode");
148+
getLogger().severe("[SpawnShield] Unknown Plugin Mode");
149149
setEnabled(false);
150150
return;
151151
}
@@ -208,8 +208,4 @@ protected void shutdown() {
208208
public SpawnShieldPlayer createPlayer(UUID id) {
209209
return new SpawnShieldPlayer(id, this);
210210
}
211-
212-
public static SpawnShield getInstance() {
213-
return JavaPlugin.getPlugin(SpawnShield.class);
214-
}
215211
}

‎base/src/main/java/net/techcable/spawnshield/SpawnShieldExecutor.java‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
*/
2323
package net.techcable.spawnshield;
2424

25+
import lombok.*;
26+
2527
import net.techcable.spawnshield.compat.ProtectionPlugin;
2628
import net.techcable.spawnshield.compat.Region;
2729
import org.bukkit.Bukkit;
@@ -31,7 +33,9 @@
3133
import org.bukkit.command.CommandExecutor;
3234
import org.bukkit.command.CommandSender;
3335

36+
@RequiredArgsConstructor
3437
public class SpawnShieldExecutor implements CommandExecutor {
38+
private final SpawnShield plugin;
3539

3640
@Override
3741
public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
@@ -72,11 +76,11 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
7276
return true;
7377
}
7478
boolean hasRegion = false;
75-
for (ProtectionPlugin plugin : SpawnShield.getInstance().getSettings().getPlugins()) {
76-
if (!plugin.hasRegion(world, regionName)) continue;;
79+
for (ProtectionPlugin protectionPlugin : plugin.getSettings().getPlugins()) {
80+
if (!protectionPlugin.hasRegion(world, regionName)) continue;;
7781
hasRegion = true;
78-
Region region = plugin.getRegion(world, regionName);
79-
SpawnShield.getInstance().getSettings().addRegionToBlock(region);
82+
Region region = protectionPlugin.getRegion(world, regionName);
83+
plugin.getSettings().addRegionToBlock(region);
8084
sender.sendMessage("Successfuly blocked region " + regionName + " in " + worldName);
8185
}
8286
if (!hasRegion) {
@@ -97,11 +101,11 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
97101
return true;
98102
}
99103
boolean hasRegion = false;
100-
for (ProtectionPlugin plugin : SpawnShield.getInstance().getSettings().getPlugins()) {
101-
if (!plugin.hasRegion(world, regionName)) continue;;
104+
for (ProtectionPlugin protectionPlugin : plugin.getSettings().getPlugins()) {
105+
if (!protectionPlugin.hasRegion(world, regionName)) continue;;
102106
hasRegion = true;
103-
Region region = plugin.getRegion(world, regionName);
104-
SpawnShield.getInstance().getSettings().addRegionToBlock(region);
107+
Region region = protectionPlugin.getRegion(world, regionName);
108+
plugin.getSettings().addRegionToBlock(region);
105109
}
106110
if (hasRegion) {
107111
sender.sendMessage("Successfuly unblocked region " + regionName + " in " + worldName);
@@ -111,7 +115,7 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
111115
return true;
112116
} else if (subSubCommand.equalsIgnoreCase("list")) {
113117
sender.sendMessage(color("&b Blocked Regions"));
114-
for (Region region : SpawnShield.getInstance().getSettings().getRegionsToBlock()) {
118+
for (Region region : plugin.getSettings().getRegionsToBlock()) {
115119
sender.sendMessage("&7Region&r " + region.getName() + " &7in world&r " + region.getWorld().getName());
116120
}
117121
return true;

‎base/src/main/java/net/techcable/spawnshield/config/SpawnShieldConfig.java‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,12 @@
2323
package net.techcable.spawnshield.config;
2424

2525
import com.google.common.base.Throwables;
26+
import com.google.common.collect.ImmutableSet;
2627
import com.google.common.collect.Lists;
2728
import com.google.common.collect.Sets;
28-
import lombok.AccessLevel;
29-
import lombok.Getter;
30-
import lombok.Synchronized;
29+
30+
import lombok.*;
31+
3132
import net.cubespace.Yamler.Config.Comments;
3233
import net.cubespace.Yamler.Config.Config;
3334
import net.cubespace.Yamler.Config.InvalidConfigurationException;
@@ -49,13 +50,13 @@
4950

5051
@Getter
5152
public class SpawnShieldConfig extends Config {
53+
private final SpawnShield plugin;
54+
55+
@SneakyThrows(InvalidConverterException.class)
5256
public SpawnShieldConfig(SpawnShield plugin) {
5357
CONFIG_FILE = new File(plugin.getDataFolder(), "config.yml");
54-
try {
55-
addConverter(BlockMode.BlockModeConverter.class);
56-
} catch (InvalidConverterException e) {
57-
throw Throwables.propagate(e);
58-
}
58+
addConverter(BlockMode.BlockModeConverter.class);
59+
this.plugin = plugin;
5960
}
6061

6162
@Override
@@ -134,7 +135,7 @@ public void refreshRegionsToBlock() {
134135
}
135136

136137
@Getter(AccessLevel.NONE)
137-
private transient Set<Region> cachedRegionsToBlock;
138+
private transient volatile ImmutableSet<Region> cachedRegionsToBlock;
138139

139140
@Synchronized("lock")
140141
public void addProtectionPlugin(ProtectionPlugin plugin) {
@@ -151,23 +152,24 @@ public Collection<Region> getRegionsToBlock() { // A devious combination of doub
151152
if (cachedRegionsToBlock != null) return cachedRegionsToBlock;
152153
synchronized (lock) {
153154
if (cachedRegionsToBlock != null) return cachedRegionsToBlock;
154-
cachedRegionsToBlock = Sets.newSetFromMap(new ConcurrentHashMap<Region, Boolean>());
155+
ImmutableSet.Builder<Region> regionsBuilder = ImmutableSet.builder();
155156
Set<String> notFound = Sets.newHashSet(blockRegions);
156157
for (ProtectionPlugin plugin : plugins) {
157158
for (World world : Bukkit.getWorlds()) {
158159
for (String regionName : blockRegions) {
159160
if (!plugin.hasRegion(world, regionName)) continue;
160161
Region region = plugin.getRegion(world, regionName);
161162
notFound.remove(regionName); //We found it !!
162-
cachedRegionsToBlock.add(region);
163+
regionsBuilder.add(region);
163164
}
164165
}
165166
}
166167
//Warn if we couldn't find a worldguard region
167168
for (String regionName : notFound) {
168-
SpawnShield.getInstance().getLogger().warning(regionName + " is not a known region");
169+
plugin.getLogger().warning(regionName + " is not a known region");
169170
}
170-
return cachedRegionsToBlock;
171+
cachedRegionsToBlock = regionsBuilder.build();
171172
}
173+
return cachedRegionsToBlock;
172174
}
173175
}

‎base/src/main/java/net/techcable/spawnshield/config/SpawnShieldMessages.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,4 @@ public String getCantEnterSafezone() {
6767
public static String color(String s) {
6868
return ChatColor.translateAlternateColorCodes('&', s);
6969
}
70-
71-
public static SpawnShieldMessages getInstance() {
72-
return SpawnShield.getInstance().getMessages();
73-
}
7470
}

‎base/src/main/java/net/techcable/spawnshield/forcefield/ForceFieldListener.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public class ForceFieldListener implements Listener {
5757
public void onMove(PlayerMoveEvent event) {
5858
if (event.getFrom().equals(event.getTo())) return; //Don't wanna fire if the player turned his head
5959
if (currentlyProcessing.contains(event.getPlayer().getUniqueId())) return;
60-
final SpawnShieldPlayer player = SpawnShield.getInstance().getPlayer(event.getPlayer());
60+
final SpawnShieldPlayer player = plugin.getPlayer(event.getPlayer());
6161
if (!plugin.getCombatTagPlugin().isTagged(event.getPlayer())) {
6262
if (player.getLastShownBlocks() != null && !currentlyProcessing.contains(player.getId())) {
6363
currentlyProcessing.add(player.getId());
@@ -70,20 +70,20 @@ public void run() {
7070
player.setLastShownBlocks(null);
7171
currentlyProcessing.remove(player.getId());
7272
}
73-
}.runTaskAsynchronously(SpawnShield.getInstance());
73+
}.runTaskAsynchronously(plugin);
7474
}
7575
return;
7676
}
7777
currentlyProcessing.add(player.getId());
7878
BlockPos pos = new BlockPos(player.getEntity().getLocation());
7979
Collection<Region> toUpdate = new HashSet<>();
80-
for (Region region : SpawnShield.getInstance().getSettings().getRegionsToBlock()) {
80+
for (Region region : plugin.getSettings().getRegionsToBlock()) {
8181
if (!region.getWorld().equals(event.getPlayer().getWorld())) continue; //We dont need this one: Yay!
8282
toUpdate.add(region);
8383
}
84-
ForceFieldUpdateRequest request = new ForceFieldUpdateRequest(pos, toUpdate, player, SpawnShield.getInstance().getSettings().getForcefieldRange());
85-
final ForceFieldUpdateTask task = new ForceFieldUpdateTask(request);
86-
Bukkit.getScheduler().runTaskAsynchronously(SpawnShield.getInstance(), task);
84+
ForceFieldUpdateRequest request = new ForceFieldUpdateRequest(pos, toUpdate, player, plugin.getSettings().getForcefieldRange());
85+
final ForceFieldUpdateTask task = new ForceFieldUpdateTask(plugin, request);
86+
Bukkit.getScheduler().runTaskAsynchronously(plugin, task);
8787
task.addListener(new Runnable() {
8888
@Override
8989
public void run() {

‎base/src/main/java/net/techcable/spawnshield/tasks/ForceFieldUpdateTask.java‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,33 +22,31 @@
2222
*/
2323
package net.techcable.spawnshield.tasks;
2424

25-
import com.google.common.collect.*;
25+
import lombok.*;
26+
27+
import java.util.Collection;
28+
import java.util.HashSet;
29+
import java.util.Map;
30+
import java.util.Set;
31+
32+
import com.google.common.collect.Maps;
2633
import com.google.common.util.concurrent.AbstractFuture;
2734
import com.google.common.util.concurrent.ListenableFuture;
28-
import lombok.RequiredArgsConstructor;
2935

3036
import net.techcable.spawnshield.SpawnShield;
3137
import net.techcable.spawnshield.compat.BlockPos;
3238
import net.techcable.spawnshield.compat.Region;
33-
import net.techcable.spawnshield.forcefield.*;
39+
import net.techcable.spawnshield.forcefield.BorderFinder;
40+
import net.techcable.spawnshield.forcefield.ForceFieldUpdateRequest;
3441

3542
import org.bukkit.Bukkit;
3643
import org.bukkit.Material;
3744

38-
import java.util.Collection;
39-
import java.util.HashSet;
40-
import java.util.Map;
41-
import java.util.Set;
42-
4345
@RequiredArgsConstructor
4446
public class ForceFieldUpdateTask extends AbstractFuture implements Runnable, ListenableFuture {
45-
46-
public static ListenableFuture<?> schedule(ForceFieldUpdateRequest request) {
47-
ForceFieldUpdateTask task = new ForceFieldUpdateTask(request);
48-
Bukkit.getScheduler().runTask(SpawnShield.getInstance(), task);
49-
return task;
50-
}
47+
private final SpawnShield plugin;
5148
private final ForceFieldUpdateRequest request;
49+
5250
@Override
5351
public void run() {
5452
Set<BlockPos> shownBlocks = new HashSet<BlockPos>();
@@ -80,8 +78,8 @@ public void run() {
8078
private final Map<Region, Collection<BlockPos>> borderCache = Maps.newHashMap(); //Will only be accessed by a single task, so no need for synchronization
8179
private Collection<BlockPos> getBorders(Region region) {
8280
if (borderCache.size() > 50) {
83-
SpawnShield.getInstance().getLogger().severe("Cache exceeded 50 entries, which should never happen.");
84-
SpawnShield.getInstance().getLogger().severe("Clearing cache");
81+
plugin.getLogger().severe("Cache exceeded 50 entries, which should never happen.");
82+
plugin.getLogger().severe("Clearing cache");
8583
borderCache.clear();
8684
}
8785
if (!borderCache.containsKey(region)) {

‎base/src/main/java/net/techcable/spawnshield/tasks/KnockbackTask.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,12 @@ public class KnockbackTask extends BukkitRunnable {
4141
@Override
4242
public void run() {
4343
for (Player playerEntity : Bukkit.getOnlinePlayers()) {
44-
SpawnShieldPlayer player = SpawnShield.getInstance().getPlayer(playerEntity);
44+
SpawnShieldPlayer player = plugin.getPlayer(playerEntity);
4545
if (isBlocked(playerEntity.getLocation())) {
4646
if (!plugin.getCombatTagPlugin().isTagged(playerEntity)) continue;
4747
if (player.getLastLocationOutsideSafezone() == null) return;
4848
if (player.getLastCantEnterMessageTime() + 1500 < System.currentTimeMillis()) {
49-
playerEntity.sendMessage(SpawnShield.getInstance().getMessages().getCantEnterSafezone());
49+
playerEntity.sendMessage(plugin.getMessages().getCantEnterSafezone());
5050
player.setLastCantEnterMessageTime(System.currentTimeMillis());
5151
}
5252
Vector knockback = player.getLastLocationOutsideSafezone().toVector().subtract(playerEntity.getLocation().toVector());
@@ -59,7 +59,7 @@ public void run() {
5959
}
6060

6161
public boolean isBlocked(Location l) {
62-
for (Region r : SpawnShield.getInstance().getSettings().getRegionsToBlock()) {
62+
for (Region r : plugin.getSettings().getRegionsToBlock()) {
6363
if (r.contains(l.getBlockX(), l.getBlockY(), l.getBlockZ())) return true;
6464
}
6565
return false;

‎base/src/main/java/net/techcable/spawnshield/tasks/TeleportSafezoningTask.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ public class TeleportSafezoningTask extends BukkitRunnable {
4141
@Override
4242
public void run() {
4343
for (Player playerEntity : Bukkit.getOnlinePlayers()) {
44-
SpawnShieldPlayer player = SpawnShield.getInstance().getPlayer(playerEntity);
44+
SpawnShieldPlayer player = plugin.getPlayer(playerEntity);
4545
if (isBlocked(playerEntity.getLocation())) {
4646
if (plugin.getCombatTagPlugin().isTagged(playerEntity)){
4747
if (player.getLastLocationOutsideSafezone() == null) {
48-
SpawnShield.getInstance().getLogger().warning(player.getName() + "'s last location outside safezone is unknown");
48+
plugin.getLogger().warning(player.getName() + "'s last location outside safezone is unknown");
4949
} else {
5050
if (player.getLastCantEnterMessageTime() + 1500 < System.currentTimeMillis()) {
51-
playerEntity.sendMessage(SpawnShieldMessages.getInstance().getCantEnterSafezone());
51+
playerEntity.sendMessage(plugin.getMessages().getCantEnterSafezone());
5252
player.setLastCantEnterMessageTime(System.currentTimeMillis());
5353
}
5454
playerEntity.teleport(player.getLastLocationOutsideSafezone(), PlayerTeleportEvent.TeleportCause.PLUGIN);
@@ -61,7 +61,7 @@ public void run() {
6161
}
6262

6363
public boolean isBlocked(Location l) {
64-
for (Region r : SpawnShield.getInstance().getSettings().getRegionsToBlock()) {
64+
for (Region r : plugin.getSettings().getRegionsToBlock()) {
6565
if (r.contains(l.getBlockX(), l.getBlockY(), l.getBlockZ())) return true;
6666
}
6767
return false;

0 commit comments

Comments
 (0)