CODEWITHSUNDEEP
javamultithreadingsynchronized
Timmos
Timmos

Reputation: 3324

Double-Checked Locking without creating objects

I'm using Double-Checked Locking (DCL) to avoid the need of synchronization on an object if not needed to do so. In my case I need to synchronize when some buffer is empty, to have the "processing thread" to wait for the "delivery thread" to notify it again - otherwise, the "processing thread" will run in loops without doing something useful.

Both threads share these objects:

Object bufferLock = new Object();
Queue<Command> commands = new ConcurrentLinkedQueue<>(); // thread safe!

Thread 1 ("delivery thread" - fills the buffer):

while (true)
    Command command = readCommand();
    commands.add(command);
    synchronize (bufferLock){
        bufferLock.notify(); // wake up Thread 2 (if waiting)
    }
}

Thread 2 ("processing thread" - empties the buffer):

while (true){
    if (commands.peek() == null){ // not creating anything here
        synchronized (bufferLock){
            if (commands.peek() == null){ // also not creating anything
                bufferLock.wait();
            }
        }
    }
    Command command = commands.poll();
    processCommand(command);
}

Now, NetBeans is showing up a warning about DCL, which made me dig into the subject deeper because the concept of DCL was unknown to me - I just started to use it on my own.

As far as I understand from reading several articles on the internet, there was a Java bug when using this pattern, but all examples use it in combination with lazy loading. In these examples, objects are created within the synchronized block. In my synchronized code, I do not create objects.

Is my code unsafe? Is NetBeans correct in showing a warning? Note that NetBeans had a bug related to DCL before, so I'm somewhat confused.

Upvotes: 1

Views: 203

Answers (2)

Neil Coffey
Neil Coffey

Reputation: 21795

The pattern (or rather, anti-pattern!) that you have created does not strictly constitute Double Checked Locking, which normally refers to the case where an object reference starts off null then is instantiated by the first thread that needs to reference it, with synchronization only after the null check. Up to Java 5, you couldn't strictly speaking implement this correctly in Java (though because of how most JVMs were implemented, you could accidentally get away with it). As of Java 5, you can use it, but it's essentially pointless. (You may be interested in an article on double-checked locking in Java that I wrote a while ago which looks at this issue in more detail. The class-loader effectively has built-in synchronisation for rare cases where you do actually need something like DCL.)

Now, that's all kind of by the by. What you have here isn't really DCL strictly-speaking.

The problem that you do have is that you're trying to mix paradigms. The raison d'être of the Java concurrency library is generally to avoid low-level locking with synchronized and wait/notify. So what you should really do is simply use some flavour of BlockingQueue and use its built-in blocking behaviour. Again, among other examples might I refer you to some material I have written on blocking queues.

Upvotes: 1

Cha2lenger
Cha2lenger

Reputation: 38

Please check the "Java Concurrency in Practice" book by Brian Goetz, section 16.2.4 "Double-checked locking". It provides the explanation of why DCL is a wrong thing to do.

Upvotes: 0

Related Questions