The server logger should be considered thread-safe by default (log4j).

To be sure that we don't kill exotic systems, it's also configurable.

NCPCore/src/main/java/fr/neatmonster/nocheatplus/config/ConfPaths.java

@@ -35,6 +35,7 @@ public abstract class ConfPaths {
     private static final String LOGGING_BACKEND_CONSOLE					 = LOGGING_BACKEND + "console.";
     public static final String  LOGGING_BACKEND_CONSOLE_ACTIVE           = LOGGING_BACKEND_CONSOLE + "active";
     public static final String  LOGGING_BACKEND_CONSOLE_PREFIX			 = LOGGING_BACKEND_CONSOLE + "prefix";
+    public static final String  LOGGING_BACKEND_CONSOLE_ASYNCHRONOUS     = LOGGING_BACKEND_CONSOLE + "asynchronous";
     private static final String LOGGING_BACKEND_FILE					 = LOGGING_BACKEND + "file.";
     public static final String  LOGGING_BACKEND_FILE_ACTIVE	        	 = LOGGING_BACKEND_FILE + "active";
     public static final String  LOGGING_BACKEND_FILE_FILENAME	         = LOGGING_BACKEND_FILE + "filename";

NCPCore/src/main/java/fr/neatmonster/nocheatplus/config/DefaultConfig.java

@@ -39,6 +39,7 @@ public class DefaultConfig extends ConfigFile {
         set(ConfPaths.LOGGING_MAXQUEUESIZE, 5000);
         set(ConfPaths.LOGGING_BACKEND_CONSOLE_ACTIVE, true);
         set(ConfPaths.LOGGING_BACKEND_CONSOLE_PREFIX, "[NoCheatPlus] ");
+        set(ConfPaths.LOGGING_BACKEND_CONSOLE_ASYNCHRONOUS, true);
         set(ConfPaths.LOGGING_BACKEND_FILE_ACTIVE, true);
         set(ConfPaths.LOGGING_BACKEND_FILE_PREFIX, "");
         set(ConfPaths.LOGGING_BACKEND_FILE_FILENAME, "nocheatplus.log");

NCPCore/src/main/java/fr/neatmonster/nocheatplus/logging/LogManager.java

@@ -60,7 +60,7 @@ public class LogManager extends AbstractLogManager {
                 return;
             }
             // Attach a new restrictive init logger.
-            // TODO: If thread-safe use ANY_THREAD_DIRECT (should then allow to interrupt).
+            boolean bukkitLoggerAsynchronous = ConfigManager.getConfigFile().getBoolean(ConfPaths.LOGGING_BACKEND_CONSOLE_ASYNCHRONOUS);
             LoggerID initLoggerID = registerStringLogger(new ContentLogger<String>() {
                 
                 @Override
@@ -70,7 +70,7 @@ public class LogManager extends AbstractLogManager {
                     } catch (Throwable t) {}
                 }
                 
-            }, new LogOptions(Streams.INIT.name, CallContext.PRIMARY_THREAD_ONLY));
+            }, new LogOptions(Streams.INIT.name, bukkitLoggerAsynchronous ? CallContext.ANY_THREAD_DIRECT : CallContext.PRIMARY_THREAD_ONLY));
             attachStringLogger(initLoggerID, Streams.INIT);
         }
     }
@@ -88,20 +88,21 @@ public class LogManager extends AbstractLogManager {
         }) {
             createStringStream(streamID);
         }
-        // TODO: Consult configuration and/or detect what options can or want to be used here.
+        // TODO: Might attempt to detect if a thread-safe logging framework is in use ("default" instead of false/true).
-        CallContext bukkitLoggerContext = CallContext.PRIMARY_THREAD_TASK; // TODO: Config + individually.
+        boolean bukkitLoggerAsynchronous = config.getBoolean(ConfPaths.LOGGING_BACKEND_CONSOLE_ASYNCHRONOUS);
         LoggerID tempID;
         
         // Server logger.
-        tempID = registerStringLogger(Bukkit.getLogger(), new LogOptions(Streams.SERVER_LOGGER.name, bukkitLoggerContext));
+        tempID = registerStringLogger(Bukkit.getLogger(), new LogOptions(Streams.SERVER_LOGGER.name, bukkitLoggerAsynchronous ? CallContext.ASYNCHRONOUS_TASK : CallContext.PRIMARY_THREAD_TASK));
         attachStringLogger(tempID, Streams.SERVER_LOGGER);
         attachStringLogger(tempID, Streams.STATUS); // Log STATUS to console "efficiently".
         
         // Plugin logger.
-        tempID = registerStringLogger(plugin.getLogger(), new LogOptions(Streams.PLUGIN_LOGGER.name, bukkitLoggerContext));
+        tempID = registerStringLogger(plugin.getLogger(), new LogOptions(Streams.PLUGIN_LOGGER.name, bukkitLoggerAsynchronous ? CallContext.ASYNCHRONOUS_TASK : CallContext.PRIMARY_THREAD_TASK));
         attachStringLogger(tempID, Streams.PLUGIN_LOGGER);
         
         // Ingame logger (assume not thread-safe at first).
+        // TODO: Thread-safe ProtocolLib-based implementation?
         tempID = registerStringLogger(new ContentLogger<String>() {
             
             @Override
@@ -126,6 +127,7 @@ public class LogManager extends AbstractLogManager {
         attachStringLogger(tempID, Streams.DEFAULT_FILE);
         attachStringLogger(tempID, Streams.STATUS); // Log STATUS to the default file.
         // Attach default file logger to init too, to log something, even if asynchronous, directly from any thread.
+        // TODO: Consider configurability of skipping, depending on bukkitLoggerAsynchronous.
         tempID = registerStringLogger(defaultFileLogger, new LogOptions(Streams.DEFAULT_FILE.name +".init", CallContext.ANY_THREAD_DIRECT));
         attachStringLogger(tempID, Streams.INIT);
         

NCPCore/src/main/java/fr/neatmonster/nocheatplus/logging/Streams.java

@@ -1,14 +1,14 @@
 package fr.neatmonster.nocheatplus.logging;
 
 /**
- * Default StreamIDs. Before the server is running, it's recommended only to log
+ * Default StreamIDs. Before the server tick loop is running, it's recommended
- * to the INIT stream, because tasks won't be processed before all plugins have
+ * only to log to the INIT stream, because tasks won't be processed before all
- * been loaded.
+ * plugins have been loaded.
  * <hr>
  * Default LoggerIDs have the same names and are atached to the stream of the
  * same name. Additionally there might be default loggers attached to several
  * streams with different name and options (e.g. DEFAULT_FILE.name + ".init"
- * allowing for direct asynchronous logging on the INIT stream).
+ * ensuring for direct asynchronous logging on the INIT stream).
  * <hr>
  * All names are processed case-insensitive within LogManager.
  * <hr>
@@ -29,8 +29,8 @@ public class Streams {
      * Default stream for initialization and shutdown, should always be
      * available (fall-back). Loggers must not use tasks, not more than primary
      * thread only is guaranteed here, might drop messages sent from other
-     * threads. By default the file logger gets attached, to allow direct
+     * threads. By default the file logger gets attached, to ensure that direct
-     * logging from other threads.
+     * logging from other threads arrives somewhere.
      */
     public static final StreamID INIT = new StreamID(defaultPrefix + "init");
     
@@ -46,32 +46,32 @@ public class Streams {
     // Raw default streams ---
     
     /**
-     * Default stream for the server logger (think of console). Might not allow
+     * Default stream for the server logger (think of console). Using log4j this
-     * asynchronous access, thus using a task in the primary thread by default.
+     * should run in an asynchronous task, configurable.
      */
     public static final StreamID SERVER_LOGGER = new StreamID(defaultPrefix + "logger.server");
     
     /**
      * Default stream for the plugin logger (Usually the server logger using a
-     * plugin-dependent prefix.). Might not allow asynchronous access, thus
+     * plugin-dependent prefix.). Using log4j this should run in an asynchronous
-     * using a task in the primary thread by default.
+     * task, configurable.
      */
     public static final StreamID PLUGIN_LOGGER = new StreamID(defaultPrefix + "logger.plugin");
     
     /**
      * Default ingame notifications (game chat), using a task in the primary
      * thread. (The implementation is not thread-safe, even if message sending
-     * was thread-safe.)
+     * was thread-safe. Thus sending is tied to the primary thread.)
      */
     public static final StreamID NOTIFY_INGAME = new StreamID(defaultPrefix + "chat.notify");
     
     /**
-     * Default file (defaults to nocheatplus.log), thread-safe.
+     * Default file (defaults to nocheatplus.log), always using an asynchronous task.
      */
     public static final StreamID DEFAULT_FILE = new StreamID(defaultPrefix + "file");
     
     /**
-     * Trace file (might lead to the default file), thread-safe.
+     * Trace file (might lead to the default file), always using an asynchronous task.
      */
     public static final StreamID TRACE_FILE = new StreamID(defaultPrefix + "file.trace");