Search code examples
javamultithreadingloggingthread-safetythreadpool

Multithreaded printing from ArrayList


I am trying to have the logger print all the log messages from the application to the console and in the future an external file. It should do this when I fire the function: "dumpToConsole". It should be done multithreaded through 3 threads that all 3 access the CopyOnWriteArrayList. The problem is that the output is not in order, and triple of what it should be. Instead of 3 messages I get 9. I need the 3 threads to all individual print 1 for example instead of each thread printing 3.

See below for my actual implementation of this.

My thread:

public class LoggingThread extends Thread {
private boolean isStopped = false;
private CopyOnWriteArrayList<LogMessage> messages;

public LoggingThread(CopyOnWriteArrayList messages) {
    this.messages = messages;
}

@Override
public void run() {
    for (int i = 0; i < messages.size(); i++) {
        writeMessageToConsole(messages.get(i).getMessageText(), messages.get(i).getLogLevel());
    }
}

private synchronized void writeMessageToConsole(String message, LogLevel logLevel) {
    System.out.println(message + " (" + logLevel + ")");
}
}

My Logger:

public class Logger implements ILogger {
private static ILogger instance = null;
private CopyOnWriteArrayList<LogMessage> messages = new CopyOnWriteArrayList<LogMessage>();
private LoggingThread thread1;
private LoggingThread thread2;
private LoggingThread thread3;

public static ILogger getInstance() {
    if (instance == null) {
        instance = new Logger();
    }

    return instance;
}

public CopyOnWriteArrayList<LogMessage> getMessages() {
    return messages;
}

public void log(Exception ex) {
    log(ex.getMessage(), LogLevel.FATAL);
}

public void log(String message, LogLevel logLevel) {
    messages.add(new LogMessage(message, logLevel));
}

public LogMessage getLastLog() {
    if(!messages.isEmpty()) {
        return messages.get(messages.size() -1);
    }

    else {
        return new LogMessage("", LogLevel.DEBUG);
    }
}

public void dumpToConsole() {
    log("TEST1", LogLevel.FATAL);
    log("TEST2", LogLevel.DEBUG);
    log("TEST3", LogLevel.FATAL);

    thread1 = new LoggingThread(this.messages);
    thread2 = new LoggingThread(this.messages);
    thread3 = new LoggingThread(this.messages);

    thread1.start();
    thread2.start();
    thread3.start();

    try {
        thread1.join();
        thread2.join();
        thread3.join();
    }

    catch (InterruptedException e) {
        log(e.getMessage(), LogLevel.FATAL);
    }

    thread1.interrupt();
    thread2.interrupt();
    thread3.interrupt();
}
}

And my message class:

public class LogMessage {
private String messageText;
private LogLevel logLevel;

public LogMessage(String messageText, LogLevel logLevel) {
    this.messageText = messageText;
    this.logLevel = logLevel;
}

public LogLevel getLogLevel() {
    return logLevel;
}

public String getMessageText() {
    return messageText;
}
}

And the result is:

TEST1 (FATAL)
TEST2 (DEBUG)
TEST3 (FATAL)
TEST1 (FATAL)
TEST1 (FATAL)
TEST2 (DEBUG)
TEST3 (FATAL)
TEST2 (DEBUG)
TEST3 (FATAL)

Solution

  • A distinct non-answer: forget about using 3 threads here.

    The problem is that the output is not in order, and triple of what it should be. Instead of 3 messages I get 9.

    Sure. Because you ask three 3 threads to do the work that a single thread should be doing, and each one does the same work again.

    So, first of all: as soon as you say "multi threading", and you talk about printing list content, then all bets regarding ordering are off. Of course, each thread will print the messages in the correct order, but you have no control whether T1 prints all messages first, or just one, then 3 from T2, whatever. The essence of "un sync'ed" threads is that: undefined order.

    Then: even when you add the necessary steps to somehow "sync up" your threads, you gain nothing from doing so.

    There is one list in memory. There is one output file (or console). Using multiple threads to fetch the values and put them into your file doesn't speed up anything! It only creates overhead, and as seen: it creates the need for a lot "sync" code.