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

Vijay Saraswat vijay at saraswat.org
Sun Jul 1 15:23:19 EDT 2012


such as using the hardhat rules implemented in X10 see Object 
initialization paper in ECOOP 2012.



On 7/1/12 2:29 PM, Jeremy Manson wrote:
> The counter field should be final, because another thread could read
> the enclosing object before it is finished being constructed (unless
> youve taken other precautions to prevent that.
>
> Jeremy
>
> On Fri, Jun 29, 2012 at 4:24 AM, Pavel Rappo <pavel.rappo at gmail.com> wrote:
>> 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.
>> _______________________________________________
>> Concurrency-interest mailing list
>> Concurrency-interest at cs.oswego.edu
>> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at cs.oswego.edu
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>




More information about the Concurrency-interest mailing list