[concurrency-interest] [Javamemorymodel-discussion] is my counter thread safe?

Pavel Rappo pavel.rappo at gmail.com
Fri Jun 29 07:24:37 EDT 2012


1. I can't see any obvious flaws in it.
2. http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/package-summary.html
(see "Memory Consistency Properties")

On 6/29/12, Li Li <fancyerii at gmail.com> wrote:
> do you mean my counter is safe?
> putIfAbsent has a happen-before semantic? any document about this? Or
> only current implementation guarantee this?
>
> On Fri, Jun 29, 2012 at 6:54 PM, Pavel Rappo <pavel.rappo at gmail.com> wrote:
>> Hi,
>>
>> This 'counter.get(key)' cannot simply return "not well constructed"
>> object. The reason is that you use 'putIfAbsent' to put it in the map.
>> Either 'get' will return 'null' or it will return perfectly valid
>> AtomicLong object. There's a happen-before edge between these two
>> actions.
>>
>> On 6/29/12, Li Li <fancyerii at gmail.com> wrote:
>>> hi all
>>>    I have read
>>> http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
>>> and know a little about Java memory model.
>>>    I want to implement a thread safe and efficient Map<String,int>
>>> counter. I found a related question in stackoverflow
>>> http://stackoverflow.com/questions/8477352/global-in-memory-counter-that-is-thread-safe-and-flushes-to-mysql-every-x-increm
>>>    my implementation is :
>>>
>>>         private ConcurrentHashMap<String, AtomicLong> counter=new
>>> ConcurrentHashMap<String, AtomicLong>();
>>>
>>>       public void addCount(String key, long count){
>>>               if(count<=0) return;
>>>               AtomicLong current = counter.get(key);
>>>                 if(current == null) {
>>>                   current=counter.putIfAbsent(key, new AtomicLong());
>>>                   if(current == null) current=counter.get(key);
>>>                 }
>>>
>>>                 assert current!=null;
>>>                 current.addAndGet(count);
>>>
>>>       }
>>>    but after I reading
>>> http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
>>> . it seems there exists a state that may fail.
>>>    if thread1 call addCount("key",2); and thread2 call
>>> addCount("key",1); at the same time.
>>>    thread1 executes AtomicLong current = counter.get(key);  and gets
>>> null.
>>>    then thread 1 execute
>>>           if(current == null) {
>>>                   current=counter.putIfAbsent(key, new AtomicLong());
>>>    as compilers/cpus may disorder, new AtomicLong() will not be null
>>> but may be well constructed.
>>>    Then thread 2 call AtomicLong current = counter.get(key); it's not
>>> null but not well constructed. then it call current.addAndGet().
>>>     Will thread 2 crash if current is not well constructed?
>>>
>>>     I also find similar implementation in a popular lib -- guava
>>> https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/util/concurrent/AtomicLongMap.java?name=v11.0-rc1
>>>     it may also fail like my codes.
>>>     e.g. thread 1 call atomic = map.putIfAbsent(key, new
>>> AtomicLong(delta));
>>>            thread 2 get a not null atomic and call it's get();
>>>
>>> public long addAndGet(K key, long delta) {
>>>     outer: for (;;) {
>>>       AtomicLong atomic = map.get(key);
>>>       if (atomic == null) {
>>>         atomic = map.putIfAbsent(key, new AtomicLong(delta));
>>>         if (atomic == null) {
>>>           return delta;
>>>         }
>>>         // atomic is now non-null; fall through
>>>       }
>>>
>>>       for (;;) {
>>>         long oldValue = atomic.get();
>>>         if (oldValue == 0L) {
>>>           // don't compareAndSet a zero
>>>           if (map.replace(key, atomic, new AtomicLong(delta))) {
>>>             return delta;
>>>           }
>>>           // atomic replaced
>>>           continue outer;
>>>         }
>>>
>>>         long newValue = oldValue + delta;
>>>         if (atomic.compareAndSet(oldValue, newValue)) {
>>>           return newValue;
>>>         }
>>>         // value changed
>>>       }
>>>     }
>>>   }
>>> _______________________________________________
>>> Javamemorymodel-discussion mailing list
>>> Javamemorymodel-discussion at cs.umd.edu
>>> https://mailman.cs.umd.edu/mailman/listinfo/javamemorymodel-discussion
>>>
>>
>>
>> --
>> Sincerely yours, Pavel Rappo.
>


-- 
Sincerely yours, Pavel Rappo.


More information about the Concurrency-interest mailing list