CODEWITHSUNDEEP
javamultithreading
Hari Krishna
Hari Krishna

Reputation: 3568

Do I acquire a lock while returning a reference

I am going through the source code of cmis API. There one of the method is defined like below.

public OperationContext getDefaultContext() {
    lock.readLock().lock();
    try {
        return defaultContext;
    } finally {
        lock.readLock().unlock();
    }
}

I would like to know, is it necessary to acquire a lock while returning a reference? if it is required, why should I acquire the lock?

Upvotes: 1

Views: 85

Answers (2)

GhostCat
GhostCat

Reputation: 140553

This simply doesn't make sense - at least when only looking at this one method in isolation.

This code probably serializes the act of fetching that context object. But there is no sense in doing that.

Unless of course there is a corresponding setter which is implemented in a similar fashion. The other answer nicely outlined how such constructs create a order for potential calls that write or read the underlying field.

But thing is: without further explanations, that implicit behavior is very much hidden. Personally, I would prefer a solution that is more explicit. For example by not using the locking approach but by making getter and setter synchronized. Or by at least putting comments into both methods explaining the thoughts behind using that lock.

Upvotes: 2

yshavit
yshavit

Reputation: 43401

One of the purposes of a lock is to establish what's known as a happens-before relationship. A computer is allowed to reorder operations, such that what you might intuitively think is "A, then B" might actually look like "A, then B" to one thread, but "B, then A" to another. One of the things synchronization does is to basically tell the computer, "you have to do A, then B."

This can lead to very confusing cases. For instance, you can see what's called a "partial write", where a thread only sees part of some action. Imagine if you set two variables on defaultContext: defaultContext.a = "a"; defaultContext.b = "b";. Including the return statement, you have three operations: reading defaultContext (to return it), and writing a and b. A thread could see them ordered as defaultContext.b = "b" ; return defaultContext; defaultContext.a = "a";. The third of those might come much later, or even never! In other words, that thread will see the write to defaultContext.b but not the write to defaultContext.a: a partial write.

In this case, A and B are writes and reads to the variable defaultLock. If multiple threads are accessing defaultLock (even if some one read it, and others only write it), then you need to synchronize, so that each thread has a fully up-to-date of defaultContext. You are telling the computer: "those writes to A and B, they really do have to come before the return."

Note that for this to work, you have to set things up a bit carefully, since any subsequent writes to defaultContext will not be synchronized. You'd have to have one thread doing all the writes to a local variable (that is, creating and initializing the default context), then write that local variable to defaultContext while holding the write lock of lock, and then no other thread can modify defaultContext after that. In that case, there's an easier way to do it: just make defaultContext a volatile field, whose writes and reads have a similar happens-before relationship. I can't speculate as to why the author of the code didn't do that.

Assuming the code you're looking at it OpenCMIS's SessionImpl, this seems to be exactly what's going on. All reads to the defaultContext are done either through the getDefaultContext method you pasted, and all writes happen through a setDefaultContext that uses the writeLock.

Upvotes: 3

Related Questions