Bleeding: Fully rework asynchronous actions execution.

This commit is contained in:
asofold 2012-09-08 23:34:49 +02:00
parent 39fb4c4b91
commit af2b9fb2b5
12 changed files with 173 additions and 248 deletions

View File

@ -20,7 +20,6 @@ import org.bukkit.plugin.PluginDescriptionFile;
import org.bukkit.plugin.java.JavaPlugin;
import fr.neatmonster.nocheatplus.checks.CheckType;
import fr.neatmonster.nocheatplus.checks.ExecuteActionsEvent;
import fr.neatmonster.nocheatplus.checks.Workarounds;
import fr.neatmonster.nocheatplus.checks.blockbreak.BlockBreakListener;
import fr.neatmonster.nocheatplus.checks.blockinteract.BlockInteractListener;
@ -223,25 +222,6 @@ public class NoCheatPlus extends JavaPlugin implements Listener {
System.out.println("[NoCheatPlus] Version " + getDescription().getVersion() + " is enabled.");
}
/**
* This event handler is used to execute the actions when a violation is detected.
*
* @param event
* the event handled
*/
@EventHandler(
priority = EventPriority.LOWEST)
final void onExecuteActions(final ExecuteActionsEvent event) {
/*
* _____ _ _ _ _
* | ____|_ _____ ___ _ _| |_ ___ / \ ___| |_(_) ___ _ __ ___
* | _| \ \/ / _ \/ __| | | | __/ _ \ / _ \ / __| __| |/ _ \| '_ \/ __|
* | |___ > < __/ (__| |_| | || __/ / ___ \ (__| |_| | (_) | | | \__ \
* |_____/_/\_\___|\___|\__,_|\__\___| /_/ \_\___|\__|_|\___/|_| |_|___/
*/
event.executeActions();
}
public void onPlayerJoinLow(final PlayerJoinEvent event) {
/*
* ____ _ _ _

View File

@ -3,16 +3,14 @@ package fr.neatmonster.nocheatplus.checks;
import java.util.HashMap;
import java.util.Map;
import org.bukkit.Bukkit;
import org.bukkit.entity.Player;
import fr.neatmonster.nocheatplus.actions.Action;
import fr.neatmonster.nocheatplus.actions.ParameterName;
import fr.neatmonster.nocheatplus.actions.types.ActionList;
import fr.neatmonster.nocheatplus.hooks.NCPExemptionManager;
import fr.neatmonster.nocheatplus.hooks.NCPHookManager;
import fr.neatmonster.nocheatplus.metrics.MetricsData;
import fr.neatmonster.nocheatplus.players.ExecutionHistory;
import fr.neatmonster.nocheatplus.utilities.TickTask;
/*
* MM'""""'YMM dP dP
@ -56,62 +54,6 @@ public abstract class Check {
public Check(final CheckType type) {
this.type = type;
}
/**
* Convenience method.
*
* @param player
* the player
* @param vL
* the violation level
* @param addedVL
* the violation level added
* @param actions
* the actions
* @return true, if the event should be cancelled
*/
protected boolean executeActions(final Player player, final double vL, final double addedVL,
final ActionList actions) {
return executeActions(new ViolationData(this, player, vL, addedVL, actions));
}
/**
* Execute some actions for the specified player.
*
* @param violationData
* the violation data
* @return true, if the event should be cancelled
*/
protected boolean executeActions(final ViolationData violationData) {
ViolationHistory.getHistory(violationData.player).log(getClass().getName(), violationData.addedVL);
try {
// Check a bypass permission.
if (violationData.bypassPermission != null)
if (violationData.player.hasPermission(violationData.bypassPermission))
return false;
// Dispatch the VL processing to the hook manager.
if (NCPHookManager.shouldCancelVLProcessing(violationData))
// One of the hooks has decided to cancel the VL processing, return false.
return false;
// Add this failed check to the Metrics data.
MetricsData.addFailed(type);
final long time = System.currentTimeMillis() / 1000L;
boolean cancel = false;
for (final Action action : violationData.getActions())
if (getHistory(violationData.player).executeAction(violationData, action, time))
// The execution history said it really is time to execute the action, find out what it is and do
// what is needed.
if (action.execute(violationData)) cancel = true;
return cancel;
} catch (final Exception e) {
e.printStackTrace();
}
return false;
}
/**
* Execute actions, possibly thread safe according to the isMainThread flag.<br>
@ -127,19 +69,14 @@ public abstract class Check {
* if the thread the main thread
* @return true, if successful
*/
public final boolean executeActionsThreadSafe(final Player player, final double VL, final double VLAdded,
final ActionList actions, final boolean isMainThread) {
final boolean cancel;
if (isMainThread)
cancel = executeActions(player, VL, VLAdded, actions);
else
// Permission check is done in the main thread.
cancel = executeActionsThreadSafe(player, VL, VLAdded, actions, type.getPermission());
return cancel;
public boolean executeActions(final Player player, final double vL, final double addedVL,
final ActionList actions, boolean isMainThread) {
// Sync it into the main thread by using an event.
return executeActions(new ViolationData(this, player, vL, addedVL, actions), isMainThread);
}
/**
* Execute actions in a thread safe manner.
* Convenience method: Execute actions from the main thread only.
*
* @param player
* the player
@ -149,27 +86,49 @@ public abstract class Check {
* the violation level added
* @param actions
* the actions
* @param bypassPermission
* the bypass permission
* @return true, if the event should be cancelled
*/
public boolean executeActionsThreadSafe(final Player player, final double vL, final double addedVL,
final ActionList actions, final String bypassPermission) {
// Sync it into the main thread by using an event.
return executeActionsThreadSafe(new ViolationData(this, player, vL, addedVL, actions, bypassPermission));
protected boolean executeActions(final Player player, final double vL, final double addedVL,
final ActionList actions) {
return executeActions(new ViolationData(this, player, vL, addedVL, actions), true);
}
/**
* Execute actions in a thread safe manner.
* Execute some actions for the specified player, only for the main thread.
*
* @param violationData
* the violation data
* @return true, if if the event should be cancelled
* @return true, if the event should be cancelled
*/
public boolean executeActionsThreadSafe(final ViolationData violationData) {
final ExecuteActionsEvent event = new ExecuteActionsEvent(violationData);
Bukkit.getPluginManager().callEvent(event);
return event.getCancel();
protected boolean executeActions(final ViolationData violationData){
return executeActions(violationData, true);
}
/**
* Execute some actions for the specified player.
*
* @param violationData
* the violation data
* @param isMainThread If this is called from within the main thread. If true, this must really be the main thread and not from synchronized code coming form another thread.
* @return true, if the event should be cancelled
*/
protected boolean executeActions(final ViolationData violationData, final boolean isMainThread) {
// Dispatch the VL processing to the hook manager (now thread safe).
if (NCPHookManager.shouldCancelVLProcessing(violationData))
// One of the hooks has decided to cancel the VL processing, return false.
return false;
final DelayedActionsExecution delayedActions = new DelayedActionsExecution(violationData);
if (isMainThread) return delayedActions.execute();
else{
TickTask.requestActionsExecution(delayedActions);
return delayedActions.hasCancel();
}
// (Design change: Permission checks are moved to cached permissions, lazily updated.)
}
/**

View File

@ -0,0 +1,62 @@
package fr.neatmonster.nocheatplus.checks;
import fr.neatmonster.nocheatplus.actions.Action;
import fr.neatmonster.nocheatplus.actions.types.CancelAction;
import fr.neatmonster.nocheatplus.metrics.MetricsData;
/**
* For scheduling actions execution. This does not check the NCPHookManager.
* <hr>
* Not put to ViolationData itself for the possibility of adding other data (might be considered though).
* @author mc_dev
*
*/
public class DelayedActionsExecution {
protected final ViolationData violationData;
protected final Action[] actions;
public DelayedActionsExecution(final ViolationData violationData) {
this.violationData = violationData;
actions = violationData.getActions();
}
/**
* Execute actions and return if cancel.
* @return
*/
public boolean execute(){
try {
ViolationHistory.getHistory(violationData.player).log(getClass().getName(), violationData.addedVL);
// Add this failed check to the Metrics data.
MetricsData.addFailed(violationData.check.type);
final long time = System.currentTimeMillis() / 1000L;
boolean cancel = false;
for (final Action action : violationData.getActions())
if (Check.getHistory(violationData.player).executeAction(violationData, action, time))
// The execution history said it really is time to execute the action, find out what it is and do
// what is needed.
if (action.execute(violationData)) cancel = true;
return cancel;
} catch (final Exception e) {
e.printStackTrace();
// On exceptions cancel events.
return true;
}
}
/**
* Check if the actions contain a cancel.
* @return
*/
public boolean hasCancel(){
for (final Action action : actions){
if (action instanceof CancelAction) return true;
}
return false;
}
}

View File

@ -1,88 +0,0 @@
package fr.neatmonster.nocheatplus.checks;
import org.bukkit.event.Event;
import org.bukkit.event.HandlerList;
/*
* MM""""""""`M dP MMP"""""""MM dP oo
* MM mmmmmmmM 88 M' .mmmm MM 88
* M` MMMM dP. .dP .d8888b. .d8888b. dP dP d8888P .d8888b. M `M .d8888b. d8888P dP .d8888b. 88d888b. .d8888b.
* MM MMMMMMMM `8bd8' 88ooood8 88' `"" 88 88 88 88ooood8 M MMMMM MM 88' `"" 88 88 88' `88 88' `88 Y8ooooo.
* MM MMMMMMMM .d88b. 88. ... 88. ... 88. .88 88 88. ... M MMMMM MM 88. ... 88 88 88. .88 88 88 88
* MM .M dP' `dP `88888P' `88888P' `88888P' dP `88888P' M MMMMM MM `88888P' dP dP `88888P' dP dP `88888P'
* MMMMMMMMMMMM MMMMMMMMMMMM
*
* MM""""""""`M dP
* MM mmmmmmmM 88
* M` MMMM dP .dP .d8888b. 88d888b. d8888P
* MM MMMMMMMM 88 d8' 88ooood8 88' `88 88
* MM MMMMMMMM 88 .88' 88. ... 88 88 88
* MM .M 8888P' `88888P' dP dP dP
* MMMMMMMMMMMM
*/
/**
* This event is to be fired to execute actions in the main thread.
*
* @author asofold
*/
public class ExecuteActionsEvent extends Event {
/** The list of the handlers of this event. */
private static final HandlerList handlers = new HandlerList();
/**
* Return the list of all the handlers of this event.
*
* @return the handler list
*/
public static HandlerList getHandlerList() {
return handlers;
}
/** The violation data. */
private final ViolationData violationData;
/** If the actions have been executed already. */
private boolean actionsExecuted = false;
/** If the event is cancelled or not. */
private boolean cancel = false;
/**
* Instantiates a new execute actions event.
*
* @param violationData
* the violation data
*/
public ExecuteActionsEvent(final ViolationData violationData) {
this.violationData = violationData;
}
/**
* Execute the actions.
*/
public void executeActions() {
if (actionsExecuted)
return;
cancel = violationData.check.executeActions(violationData);
actionsExecuted = true;
}
/**
* Return if the event is cancelled.
*
* @return the cancellation state
*/
public boolean getCancel() {
return cancel;
}
/* (non-Javadoc)
* @see org.bukkit.event.Event#getHandlers()
*/
@Override
public HandlerList getHandlers() {
return handlers;
}
}

View File

@ -28,9 +28,6 @@ public class ViolationData {
/** The violation level added. */
public final double addedVL;
/** The bypassing permission. */
public final String bypassPermission;
/** The check. */
public final Check check;
@ -56,33 +53,11 @@ public class ViolationData {
*/
public ViolationData(final Check check, final Player player, final double vL, final double addedVL,
final ActionList actions) {
this(check, player, vL, addedVL, actions, null);
}
/**
* Instantiates a new violation data.
*
* @param check
* the check
* @param player
* the player
* @param vL
* the violation level
* @param addedVL
* the violation level added
* @param actions
* the actions
* @param bypassPermission
* the permission to bypass the execution, if not null
*/
public ViolationData(final Check check, final Player player, final double vL, final double addedVL,
final ActionList actions, final String bypassPermission) {
this.check = check;
this.player = player;
this.vL = vL;
this.addedVL = addedVL;
this.actions = actions;
this.bypassPermission = bypassPermission;
}
/**

View File

@ -50,7 +50,7 @@ public class Color extends AsyncCheck {
data.colorVL++;
// Find out if we need to remove the colors or not.
if (executeActionsThreadSafe(player, data.colorVL, 1D, cc.colorActions, isMainThread))
if (executeActions(player, data.colorVL, 1D, cc.colorActions, isMainThread))
// Remove color codes.
return message.replaceAll("\302\247.", "").replaceAll("\247.", "");
}

View File

@ -194,7 +194,7 @@ public class GlobalChat extends AsyncCheck implements INotifyReload{
}
else{
data.globalChatVL += accumulated / 10.0;
if (executeActionsThreadSafe(player, data.globalChatVL, accumulated / 10.0, cc.globalChatActions, isMainThread))
if (executeActions(player, data.globalChatVL, accumulated / 10.0, cc.globalChatActions, isMainThread))
cancel = true;
}
}

View File

@ -202,7 +202,7 @@ public class NoPwnage extends AsyncCheck implements ICaptcha{
data.noPwnageVL.add(now, (float) (suspicion / 10D));
// Find out if we need to kick the player or not.
cancel = executeActionsThreadSafe(player, data.noPwnageVL.getScore(cc.noPwnageVLFactor), suspicion / 10D, cc.noPwnageActions,
cancel = executeActions(player, data.noPwnageVL.getScore(cc.noPwnageVLFactor), suspicion / 10D, cc.noPwnageActions,
isMainThread);
}
// else
@ -238,7 +238,7 @@ public class NoPwnage extends AsyncCheck implements ICaptcha{
// Does he failed too much times?
if (data.noPwnageCaptchTries > cc.noPwnageCaptchaTries) {
// Find out if we need to kick the player or not.
executeActionsThreadSafe(player, data.captchaVL, 1, cc.noPwnageCaptchaActions,
executeActions(player, data.captchaVL, 1, cc.noPwnageCaptchaActions,
isMainThread);
// (Resetting captcha tries is done on quit/kick).
}
@ -334,7 +334,7 @@ public class NoPwnage extends AsyncCheck implements ICaptcha{
data.noPwnageReloginWarnings++;
} else if (now - data.noPwnageReloginWarningTime < cc.noPwnageReloginWarningTimeout)
// Find out if we need to ban the player or not.
cancel = executeActionsThreadSafe(player, (double) data.noPwnageVL.getScore(cc.noPwnageVLFactor), 0D, cc.noPwnageActions, true);
cancel = executeActions(player, (double) data.noPwnageVL.getScore(cc.noPwnageVLFactor), 0D, cc.noPwnageActions, true);
}
// Store his joining time.

View File

@ -74,6 +74,8 @@ public class APIUtils {
/**
* Return if the check type requires synchronization.
* <hr>
* The should be chat checks, currently.
*
* @param type
* the check type

View File

@ -14,7 +14,10 @@ import fr.neatmonster.nocheatplus.checks.CheckType;
* MMMMMMMMMMM MMMMMMMMMMM MMMMMMMMMMMM MMMMMMMMMMMM
*/
/**
* Compatibility hooks have to implement this.
* Compatibility hooks have to implement this.<br>
* NOTES:
* Some checks run asynchronously, the hooks using these also have to support processing in an extra thread, check with APIUtils.needsSynchronization(CheckType).
* Hooks that can be called asynchronously must not register new hooks that might run asynchronously during processing (...).
*
* @author asofold
*/

View File

@ -2,6 +2,7 @@ package fr.neatmonster.nocheatplus.hooks;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.LinkedList;
@ -48,6 +49,14 @@ public final class NCPHookManager {
/** Mapping the check types to the hooks. */
private static final Map<CheckType, List<NCPHook>> hooksByChecks = new HashMap<CheckType, List<NCPHook>>();
static{
// Fill the map to be sure that thread safety can be guaranteed.
for (final CheckType type : CheckType.values()){
if (APIUtils.needsSynchronization(type)) hooksByChecks.put(type, Collections.synchronizedList(new ArrayList<NCPHook>()));
else hooksByChecks.put(type, new ArrayList<NCPHook>());
}
}
/**
* Register a hook for a specific check type (all, group, or an individual check).
*
@ -93,12 +102,8 @@ public final class NCPHookManager {
* the hook
*/
private static void addToMapping(final CheckType checkType, final NCPHook hook) {
List<NCPHook> hooks = hooksByChecks.get(checkType);
if (hooks == null) {
hooks = new ArrayList<NCPHook>();
hooks.add(hook);
hooksByChecks.put(checkType, hooks);
} else if (!hooks.contains(hook))
final List<NCPHook> hooks = hooksByChecks.get(checkType);
if (!hooks.contains(hook))
hooks.add(hook);
}
@ -315,15 +320,9 @@ public final class NCPHookManager {
*/
private static void removeFromMappings(final NCPHook hook, final Integer hookId) {
allHooks.remove(hookId);
final List<CheckType> rem = new LinkedList<CheckType>();
for (final CheckType checkId : hooksByChecks.keySet()) {
final List<NCPHook> hooks = hooksByChecks.get(checkId);
if (hooks.remove(hook))
if (hooks.isEmpty())
rem.add(checkId);
hooksByChecks.get(checkId).remove(hook);
}
for (final CheckType checkId : rem)
hooksByChecks.remove(checkId);
}
/**
@ -407,10 +406,18 @@ public final class NCPHookManager {
public static final boolean shouldCancelVLProcessing(final ViolationData violationData) {
// Checks for hooks registered for this event, parent groups or ALL will be inserted into the list.
// Return true as soon as one hook returns true. Test hooks, if present.
final List<NCPHook> hooksCheck = hooksByChecks.get(violationData.check.getType());
if (hooksCheck != null)
if (applyHooks(violationData.check.getType(), violationData.player, hooksCheck))
return true;
final CheckType type = violationData.check.getType();
final List<NCPHook> hooksCheck = hooksByChecks.get(type);
if (!hooksCheck.isEmpty()){
if (APIUtils.needsSynchronization(type)){
synchronized (hooksCheck) {
return applyHooks(type, violationData.player, hooksCheck);
}
}
else{
return applyHooks(type, violationData.player, hooksCheck);
}
}
return false;
}
}

View File

@ -2,6 +2,8 @@ package fr.neatmonster.nocheatplus.utilities;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import org.bukkit.Bukkit;
@ -9,14 +11,16 @@ import org.bukkit.entity.Player;
import fr.neatmonster.nocheatplus.NoCheatPlus;
import fr.neatmonster.nocheatplus.checks.CheckType;
import fr.neatmonster.nocheatplus.checks.DelayedActionsExecution;
import fr.neatmonster.nocheatplus.checks.ICheckData;
/**
* Task to run every tick, to update permissions, and maybe later for extended lag measurement.
* Task to run every tick, to update permissions and execute actions, and maybe later for extended lag measurement.
* @author mc_dev
*
*/
public class TickTask implements Runnable {
protected static final class PermissionUpdateEntry{
public CheckType checkType;
public String playerName;
@ -41,28 +45,51 @@ public class TickTask implements Runnable {
/** Permissions to update: player name -> check type. */
private static final Set<PermissionUpdateEntry> permissionUpdates = Collections.synchronizedSet(new HashSet<PermissionUpdateEntry>(50));
/** Actions to execute. */
public static final List<DelayedActionsExecution> delayedActions = Collections.synchronizedList(new LinkedList<DelayedActionsExecution>());
/** Task id of the running TickTask */
protected static int taskId = -1;
/**
* Access method to request permisison updates.
* @param playerName
* @param checkType
*/
public static void requestPermissionUpdate(final String playerName, final CheckType checkType){
permissionUpdates.add(new PermissionUpdateEntry(playerName, checkType));
}
protected static int taskId = -1;
public static void cancel(){
if (taskId == -1) return;
Bukkit.getScheduler().cancelTask(taskId);
taskId = -1;
}
public static int start(final NoCheatPlus plugin){
cancel();
taskId = Bukkit.getScheduler().scheduleSyncRepeatingTask(plugin, new TickTask(), 1, 1);
return taskId;
private void executeActions() {
synchronized (delayedActions) {
for (final DelayedActionsExecution actions : delayedActions){
actions.execute();
}
}
}
public static void requestActionsExecution(final DelayedActionsExecution actions) {
delayedActions.add(actions);
}
@Override
public void run() {
// The isEmpty checks are faster than synchronizing fully always, the actions get delayed one tick at most.
if (!delayedActions.isEmpty()) executeActions();
if (!permissionUpdates.isEmpty()) updatePermissions();
}
public static int start(final NoCheatPlus plugin){
cancel();
taskId = Bukkit.getScheduler().scheduleSyncRepeatingTask(plugin, new TickTask(), 1, 1);
return taskId;
}
/**
* Only call from the main thread!
@ -83,7 +110,5 @@ public class TickTask implements Runnable {
permissionUpdates.clear();
}
}
}