CODEWITHSUNDEEP
javamultithreadingjunitsynchronized
Hooli
Hooli

Reputation: 1195

test if a method behaves in a synchronized way

The below code is a snippet outlining the objective of this unit test. The below demonstrates createFile only performing one task which is already known to be a thread-safe operation.

The idea therefore is more around the test than the actual operation; to prove beyond a shadow of a doubt, the behavior of a thread-safe method in that it behaves in a way that we have already proven historically.

public static final synchronized void createFile(final File file) throws IOException {
    file.createNewFile();
}

@Test
public void testCreateFileThreadSafety() throws Exception {
    for (int i = 1; i < 50; i++) {
        new Thread(new Runnable() {
            @Override
            public void run() {
                try {
                    createFile(new File(i+".txt"));
                    new File(i+".txt").delete();
                } catch (IOException ex) {
                   System.out.println(ex.getMessage());
                }
            }
        }).start();
        assertTrue(file.getParentFile().listFiles().length == 0);
    }
}

EDIT:

What's happening now: Thread gets created, file gets created, file gets deleted, thread dies, assert proves nothing and repeat

What's expected: Threads should all start and assert should ensure that only one file gets created at a time and that the other threads are waiting, not executing the method

DOUBLE EDIT:

What I really need is a refactor of the above unit test so that it does what it's supposed to do (as mentioned above)

Upvotes: 3

Views: 2890

Answers (2)

GeertPt
GeertPt

Reputation: 17864

Of course for this very simple use case, it's quite silly, because the synchronized keyword is right there. But in general, if you want to test if a method is never called concurrently, you can throw in this:

static AtomicInteger c = new AtomicInteger();

public void knownNonThreadSafeMethod(final File file) throws IOException {
    int t = c.incrementAndGet();
    doSomething();   
    Thread.yield(); //try to force race conditions, remove in prod
    assert t == c.intValue();
}

If you'd use a simple int i.s.o. the AtomicInteger, compiler optimizations would remove the assertion. 

static int c = 0;

public void knownNonThreadSafeMethod(final File file) throws IOException {
    int t = ++c;
    doSomething();   
    assert t == c; //smart-ass compiler will optimize to 'true'
}

With AtomicInteger, the value is guaranteed to be synchronised over all CPU's and all threads, and thus you'll detect any concurrent accesses.

I know it's not in a JUnit test, but I couldn't find any non-intrusive way to fix this. Maybe you can inject it via AspectJ?

Upvotes: 1

hoaz
hoaz

Reputation: 10161

Create File subclass which overrides createNewFile method like this:

class TestFile extends File {

    private final AtomicBoolean isCreated = new AtomicBoolean(false);
    private boolean isSynchronizationFailed = false;

    public boolean createNewFile() throws IOException {
        if (isCreated.compareAndSet(false, true)) {
            // give other threads chance to get here
            try {
                Thread.sleep(1000L);
            } catch (InterruptedException e) {
            }
            // cleanup
            isCreated.set(false);
        } else {
            isSynchronizationFailed = true;
        }
        return super.createNewFile();
    }
}

Pass instance of this class to your threads

Assert at the end your test that isSynchronizationFailed is false.

If two threads somehow enter createNewFile method at the same time, you will have isSynchronizationFailed variable set to true.

Upvotes: 1

Related Questions