Multithreaded log test

Question

I'm writing a logger extension that allows multiple threads to log a process, and then dump that log to the main log in one atomic operation. The point of it is to make the logs easier to read when many threads are executing. Is this test valid and clear or not?

/**
 * Test that thread logs do not interlace
 * @throws InterruptedException 
 */
@Test
public void testDumpMultithreaded() throws InterruptedException {
    Thread t1 = new Thread() {
        @Override
        public void run() {
            logger.bufferedMessages.get().add("t1: One");
            logger.bufferedMessages.get().add("t1: Two");
            logger.bufferedMessages.get().add("t1: Three");
            logger.dump();
        }
    };

    Thread t2 = new Thread() {
        @Override
        public void run() {
            logger.bufferedMessages.get().add("t2: One");
            logger.bufferedMessages.get().add("t2: Two");
            logger.bufferedMessages.get().add("t2: Three");
            logger.dump();
        }
    };

    t1.start();
    t2.start();

    t1.join();
    t2.join();

    Iterator<String> i = logger.messages.iterator();
    boolean t1Correct = false;
    boolean t2Correct = false;

    while (i.hasNext()) {
        if (i.next().equals("t1: One")) {
            t1Correct = true;
            t2Correct &= i.next().equals("t1: Two");
            t2Correct &= i.next().equals("t1: Three");
        }
    }

    i = logger.messages.iterator();
    while (i.hasNext()) {
        if (i.next().equals("t2: One")) {
            t2Correct = true;
            t2Correct &= i.next().equals("t2: Two");
            t2Correct &= i.next().equals("t2: Three");
        }
    }

    assertEquals("Thread one's log not consecutive: ", true, t1Correct);
    assertEquals("Thread two's log not consecutive: ", true, t2Correct);
}

Answer 1

A barrier can ensure the threads execute in an interleaved fashion.

final CyclicBarrier rendezvous = new CyclicBarrier(2);
final CyclicBarrier conclusion = new CyclicBarrier(3);
Thread a = new Thread() {
    public void run() {
         try {
             rendezvous.await();
             // do your stuff
             // do your stuff
             rendezvous.await(); // if you want to be extra sure the ops are interleaved
             // do your stuff
         } catch (...) {}
         finally { conclusion.await(); }
    }
};

Thread b = new Thread() {
    public void run() {
         try {
             rendezvous.await();
             // do your other stuff
             // do your other stuff
             rendezvous.await(); // if you want to be extra sure the ops are interleaved
             // do your other stuff
         } catch (...) {}
         finally { conclusion.await(); }
    }
};

a.start();
b.start();
conclusion.await();    

assertTrue instead of assertEquals(.., true, ..).

cut and paste error has you modifying t2Correct instead of t1Correct

Answer 2

The test in the OP and Ron's test are both valid tests, but it's important to realize that they may not fail even if there are legitimate concurrency problems.

The test in the OP is unlikely to ever interleave the calls to add(). Even if the test ran thousands of times, t1 would usually always finish before t2 started.

Ron's test ensures that the calls to add() do interleave by using a CyclicBarrier. However, using the CyclicBarrier ensures that the state maintained by the two threads gets flushed to main memory, potentially hiding concurrency problems in both add() and dump().

Both tests are decent, and you might as well run them both. For true peace of mind though, why not post the code for the logger on this site?