SemanticLogger.sync! is Not Thread-Safe

[eviljoel]

SemanticLogger.sync! is not thread-safe because Formatters make the assumption they will be used synchronously and thus have instance data. Honestly, I'm not sure if sync! is intended to be thread-safe or not, but the documentation does not clearly state this one way or the other. Either the code should be updated to be thread-safe or the documentation should clearly state that sync! is not thread-safe.

Specifically, the Formatter constructors make it obvious that a single instance cannot be called in a thread-safe manner. In lib/semantic_logger/formatters/default.rb, you can see that instance data is immediately saved:

      def call(log, logger)
        self.log    = log
        self.logger = logger

You can see that all threads share the same Processor instance by looking in lib/semantic_logger/logger.rb:

    def log(log, message = nil, progname = nil, &block)
      # Compatibility with ::Logger
      return add(log, message, progname, &block) unless log.is_a?(SemanticLogger::Log)

      Logger.call_subscribers(log)

      Logger.processor.log(log)
    end

By looking in lib/semantic_logger/appender/io.rb and lib/semantic_logger/subscriber.rb, you can see that a single instance of a Formatter is intended to be shared for all log calls:

      def log(log)
        # Since only one appender thread will be writing to the file at a time
        # it is not necessary to protect access to the file with a semaphore
        # Allow this logger to filter out log levels lower than it's own
        @io.write(formatter.call(log, self) << "\n")
        true
      end

    def formatter=(formatter)
      @formatter =
        if formatter.nil?
          respond_to?(:call) ? self : default_formatter
        else
          Formatters.factory(formatter)
        end
    end

If the operating system scheduler switches to another thread during the Formatter.call method and the other thread also makes a logger call, the log and logger instance values in the Formatter become corrupted for the original thread.

Again, I am requesting that either the code be updated to be thread-safe or the documentation be updated to state that sync! should only be used in single threaded applications.

Environment

Provide at least:

• Semantic Logger Version: master branch

Expected Behavior

Log entries are printed with the correct Log object.

Actual Behavior

Log entries are printed with a Log object from another thread.

Pull Request

It isn't immediately clear what the intended behavior is.