river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Firmstone <j...@zeus.net.au>
Subject Re: Possible multi-threading bug
Date Tue, 30 Nov 2010 09:43:41 GMT
Patricia Shanahan wrote:
> Tom Hobbs wrote:
>> Yes, you're right.
>>
>> I knew about the non-atomicity of ++, my concern was a call to reset
>> creeping in between the two parts of that operation.
>
> That is a very good point.
>
> Leaving reset unsynchronized, even with volatile, would lead to 
> results that would not be possible with full synchronization. Suppose 
> thread A is going to do a reset, and thread B is going to do an 
> increment, and everybody agrees the count is currently 10.
>
> With synchronization, after both operations have completed, count is 
> either 1 (A:reset, B:increment) or 0 (B:increment, A:reset).
>
> With only increment synchronized, even if the the count is volatile, 
> we add the possible outcome 11, which cannot be reached by executing 
> the two methods atomically:
>
> B: volatile read, result 10
> A: volatile write of 0
> B: volatile write of 11
>
> I think it is best to stick to synchronization and the 
> java.util.concurrent classes, ignoring volatile, except in really 
> important performance-critical situations. If we do find we need to 
> use volatile, I would like to take the time to do a formal 
> proof-of-correctness based on the JLS rules.
>
>> I forgot about the "Get a global lock on the variable" that volatile
>> would require as mentioned here;
>> http://www.javaperformancetuning.com/news/qotm051.shtml
>
> I don't think we can depend on anything other than the JLS, or 
> possibly "Java Concurrency in Practice" which seems to tend towards 
> safety if it simplifies the situation at all.
>
> For example, I would be surprised if a SPARC v.9 TSO implementation of 
> volatile needed to do more than a "membar #StoreLoad" between a store 
> to a volatile variable and the next load of anything. Even on a system 
> that needed to do more, any locking would treat the volatile read and 
> the volatile write of the ++ as two separate operations.
>
> Patricia
>

Ah yes, correct, my mistake, easy to stuff up isn't it? ;)

In essence I agree, unfortunately we don't know when we need the 
performance, because we can't test scalability.  I've only got 4 threads!

So we can just use the concurrency utilities if were worried about 
synchronization performance.

Here are some simple tips I've found useful:

   1. If all mutators are atomic and don't depend on previous state, a
      volatile reference or field may be sufficient, but now we have
      concurrency utilities, why not use an atomic reference or field
      instead? Then if we find we later need a method based on previous
      state, it's easily proven correct.
   2. If any mutator depends on previous state, all mutator methods need
      to synchronized on the same lock, volatile is acceptable for
      reads, but now we have ReentrantReadWrite lock, why bother?
   3. If using implicit synchronization, make all methods synchronized,
      otherwise use explicit synchronization instead, it's much clearer
      what your lock protects.
   4. Builders, Immutable objects and atomic references are good for
      keeping mutable state isolated to a single thread, publish the
      immutable object.  Eg StringBuilder and String.
   5. Care must also be taken to distinguish between synchronization on
      a reference and the object it references.
   6. When using ConcurrentHashMap, always check the result of put if
      absent and update your local reference if you need to.
   7. We need also to be careful of livelock or deadlock due to excess
      synchronization.
   8. Sometimes its better not to use synchronization at all, just don't
      share the object between threads, pass it and don't retain a
      reference.
   9. Don't call external methods from inside a synchronization block,
      this can lead to deadlock.


Peer review is good for this reason, it's easy to get it wrong.

Keeping Objects simple makes peer review easier.

Shared mutable state is tough.

Well spotted Patricia.

Have you got any examples of a formal proof of correctness?  Just out of 
curiosity?

Cheers,

Peter.


Mime
View raw message