By: David Wragg (david.delete@this.wragg.org), October 20, 2006 2:46 am
Room: Moderated Discussions
Tzvetan Mikov (tmikov@gmail.com) on 10/19/06 wrote:
---------------------------
>Another thread could see the write to m_cachedResult before it has seen the writes
>to newObject. So from that thread, m_cachesResult will point to an uninitialized object.
>
>This could be viewed as a programming error, but the bigger problem is that m_cachedResult
>could actually be seen as pointing to uninitialized memory from a second
>thread. This is no longer a programming error - it violates the safety invariants
>of the language and it could cause the JVM to crash.
Your basic point is correct - another thread might find the object in an uninitialized state. When I was a postgraduate, a fellow PhD student (Vishnu Kotrajaras) discussed this in his thesis.
The JSR133 revision to the Java memory model changes things for final fields, but not for non-final fields, as far as I remember.
Java specifies default values for unitialized fields: 0 for ints, null for pointers, etc. An implementation can obey this by allocating new objects from zeroed blocks (with appropriate barriers around the zeroing process). The problem is the object header, particularly the vtable pointer. On an architecture with a suitably relaxed memory ordering, a thread might get the address of an object, read a zero vtable address, and then try to access the vtable (e.g. for a method call), leading to a SIGSEGV. But as part of its many duties, the SIGSEGV handler can detect this rare case and fix it up (probably by resuming execution at an appropriate safepoint).
I don't think Sun's JVM actually does any of this, and instead relies on the architecture to provide a reasonably strict memory ordering.
Your basic point stands, and your example Java code should be considered buggy: The caller might find that it gets a valid object back, but all the fields are null, 0, etc. It does need to synchronize, though not necessarily during expensive initialization activity. E.g.
The advantage is that you don't hold the lock for a long time, in order to avoid contending with other uses of the same lock. Of course, at that point you might well decide to use a dedicated lock and synchronize over the whole method, to ensure that you don't create redundant objects.
---------------------------
>Another thread could see the write to m_cachedResult before it has seen the writes
>to newObject. So from that thread, m_cachesResult will point to an uninitialized object.
>
>This could be viewed as a programming error, but the bigger problem is that m_cachedResult
>could actually be seen as pointing to uninitialized memory from a second
>thread. This is no longer a programming error - it violates the safety invariants
>of the language and it could cause the JVM to crash.
Your basic point is correct - another thread might find the object in an uninitialized state. When I was a postgraduate, a fellow PhD student (Vishnu Kotrajaras) discussed this in his thesis.
The JSR133 revision to the Java memory model changes things for final fields, but not for non-final fields, as far as I remember.
Java specifies default values for unitialized fields: 0 for ints, null for pointers, etc. An implementation can obey this by allocating new objects from zeroed blocks (with appropriate barriers around the zeroing process). The problem is the object header, particularly the vtable pointer. On an architecture with a suitably relaxed memory ordering, a thread might get the address of an object, read a zero vtable address, and then try to access the vtable (e.g. for a method call), leading to a SIGSEGV. But as part of its many duties, the SIGSEGV handler can detect this rare case and fix it up (probably by resuming execution at an appropriate safepoint).
I don't think Sun's JVM actually does any of this, and instead relies on the architecture to provide a reasonably strict memory ordering.
Your basic point stands, and your example Java code should be considered buggy: The caller might find that it gets a valid object back, but all the fields are null, 0, etc. It does need to synchronize, though not necessarily during expensive initialization activity. E.g.
public Result getResult ()
{
synchronized (this) {
if (m_cachedResult == null)
return m_cachedResult;
}
Result newObject = new Result();
expensiveResultInitialization(newObject);
synchronized (this) {
m_cachedResult = newObject;
}
return m_cachedResult;
}
The advantage is that you don't hold the lock for a long time, in order to avoid contending with other uses of the same lock. Of course, at that point you might well decide to use a dedicated lock and synchronize over the whole method, to ensure that you don't create redundant objects.