ConcurrentHashMap reorder instruction?

Question

I'm looking into ConcurrentHashMap implementation and have a thing make me confused.

/* Specialized implementations of map methods */

        V get(Object key, int hash) {
            if (count != 0) { // read-volatile
                HashEntry<K,V> e = getFirst(hash);
                while (e != null) {
                    if (e.hash == hash && key.equals(e.key)) {
                        V v = e.value;
                        if (v != null)
                            return v;
                        return readValueUnderLock(e); // recheck
                    }
                    e = e.next;
                }
            }
            return null;
        }

and

    /**
     * Reads value field of an entry under lock. Called if value
     * field ever appears to be null. This is possible only if a
     * compiler happens to reorder a HashEntry initialization with
     * its table assignment, which is legal under memory model
     * but is not known to ever occur.
     */
    V readValueUnderLock(HashEntry<K,V> e) {
        lock();
        try {
            return e.value;
        } finally {
            unlock();
        }
    }

and HashEntry constructor

/**
     * ConcurrentHashMap list entry. Note that this is never exported
     * out as a user-visible Map.Entry.
     *
     * Because the value field is volatile, not final, it is legal wrt
     * the Java Memory Model for an unsynchronized reader to see null
     * instead of initial value when read via a data race.  Although a
     * reordering leading to this is not likely to ever actually
     * occur, the Segment.readValueUnderLock method is used as a
     * backup in case a null (pre-initialized) value is ever seen in
     * an unsynchronized access method.
     */
    static final class HashEntry<K,V> {
    final K key;
            final int hash;
            volatile V value;
            final HashEntry<K,V> next;

            HashEntry(K key, int hash, HashEntry<K,V> next, V value) {
                this.key = key;
                this.hash = hash;
                this.next = next;
                this.value = value;
            }

put implement

tab[index] = new HashEntry<K,V>(key, hash, first, value);

I confused at HashEntry comment, as JSR-133, once HashEntry is constructed, all final fields will be visible to all other threads, value field is volatile, so I think it visible to other threads too??? . Other point, is the reorder he said is: HashEntry object reference can be assigned to tab[...] before it is full constructed (so result is other threads can see this entry but e.value can be null) ?

Update: I read this article and it's good. But do I need to care about a case like this

ConcurrentLinkedQueue queue = new ConcurrentLinkedQueue();

thread1:

Person p=new Person("name","student");        
queue.offer(new Person());

thread2:
Person p = queue.poll();

Is there a chance that thread2 receive an unfinished-construct Person object just like HashEntry in

tab[index] = new HashEntry(key, hash, first, value); ?

Answer 1

For those interested in an answer from the Doug Lea on this topic, he recently explained the reason for readValueUnderLock

This is in response to someone who had the question:

In the ConcurrentHashMap the get method does not require "readValueUnderLock" because a racing remove does not make the value null. The value never becomes null on the from the removing thread. this means it is possible for get to return a value for key even if the removing thread (on the same key) has progressed till the point of cloning the preceding parts of the list. This is fine so long as it is the desired effect.

But this means "readValueUnderLock" is not required for NEW memory model.

However for the OLD memory model a put may see the value null due to reordering(Rare but possible).

Is my understanding correct.

Response:

Not quite. You are right that it should never be called. However, the JLS/JMM can be read as not absolutely forbidding it from being called because of weaknesses in required ordering relationships among finals vs volatiles set in constructors (key is final, value is volatile), wrt the reads by threads using the entry objects. (In JMM-ese, ordering constraints for finals fall outside of the synchronizes-with relation.) That's the issue the doc comment (pasted below) refers to. No one has ever thought of any practical loophole that a processor/compiler might find to produce a null value read, and it may be provable that none exist (and perhaps someday a JLS/JMM revision will fill in gaps to clarify this), but Bill Pugh once suggested we put this in anyway just for the sake of being conservatively pedantically correct. In retrospect, I'm not so sure this was a good idea, since it leads people to come up with exotic theories.

It can all be viewed here

Answer 2

I confused at HashEntry comment, as JSR-133, once HashEntry is constructed, all final fields will be visible to all other threads, value field is volatile, so I think it visible to other threads too??? .

Other threads will see value as well but... The assigment of the entry (into the Object[]) is done after the initialization AND under lock. So if any thread sees null it will try to read the value under the lock.

Other point, is the reorder he said is: HashEntry object reference can be assigned to tab[...] before it is full constructed (so result is other threads can see this entry but e.value can be null) ?

No, it cannot b/c there is a volatile assignment (value) and than means all other operations must be set before hand (i.e. not reordered). Also keep in mind that java object creation is 2 phase, creating an empty object w/ zero/null fields (like using the default c-tor) and then calling <init> method (which is the constructor). The object cannot be assigned to anything before completing the constructor call and its last assignment of value (to ensure proper ordering also known as happens-before)