Ticket #1980 (closed defect: fixed)

See also  this thread on imagej-devel.

It might be best to make methods like this:

public LogService log() {
    if (log == null) synchronized (this) {
        if (log == null) {
            log = getContext().getService(LogService.class);
        }
    }
    return log;
}

That way, we avoid races (by having a check for log == null in the synchronized block) but we also avoid the cost of the synchronized block in the vast majority of the cases, see  http://norvig.com/java-iaq.html#slow

Thanks dscho. I take back what I said about it being tricky. I even thought about that approach while working on the lazy initialization logic originally, but thought "YAGNI" as well as "it would be ugly", but clearly we *do* need it and it is very straightforward. As for it being ugly, dscho also demonstrated that  it can be written more elegantly.

So we will definitely make similar such changes to the SciJava Common and ImageJ2 core services.

1) Double checked locking works after Java 1.5 if you make 'log' here volatile.

 https://en.wikipedia.org/wiki/Double-checked_locking

2) Dscho's Norvig link has "Using bitset.get(i) is 60 times slower than bits & 1 << i. This is the cost of a synchronized method call, mostly." However "bits & 1 << i" is an and, a shift, and a few loads, so just a few clock cycles. 60 times that is still negligible.

Thanks aivar. I take back what I said about taking back what I said about it being tricky. Why is multithreading always so complicated??

Anyway, we can still fix this of course, using volatile and as described on that wiki page. It's just more complex than I would like.

The inlining of constructors is the problem here. But getContext().getService(...) cannot be inlined, because getService() call performs a real lookup (and hence cannot assign the variable early).

So while it is true that double-checked locking has its problems, only little more research is needed in this case to point out that the pointed out problem is in fact not a problem. Just wanted to point that out.

We use the double checked locking pattern for all services now, AFAIK.