[b]Hibernate version:[/b]

3.0.2


This is more of a thready saftey question. In class org.hibernate.stat.StatisticsImpl, I noticed that synchronized is being used to update values but not when getting values. E.g

public synchronized void connect() {
connectCount++;
}

public long getConnectCount() {
return connectCount;
}


From my understanding of threading, this is not correct thready safety code. This is especially true when values are being accessed via MBeans. Comments?

Dino

statistic numbers does not need to be 100% precise when read, but we need to ensure values are not "lost" when writing.

Having synchronized on write gives safety and no synchronize on read gives less overhead

I understand. But what happens if we are using these values for monitoring? This could be a problem on multi-cpu machines where one thread may cache the results and may not resync with the main memory values. Don't we want to display correct results all the time?

Dino

since writes are synchronized the data will always be 100% correct - but you might have read the value 1 milisecond before it was updated, and that would also happen if read was synchronized.

Doug Lea wrote this to me about this topic:
==================

It is just plain wrong. A JVM is allowed to reorder/cache
per-thread reads unless they are loaded under sync or
are volatile (or also, java.util.concurrent.atomic's). So
a reading thread can see an arbitrarily old value. In fact
it can always return zero to any thread that did not itself
update it. JVMs tend not to exploit this as much as they are
allowed to, so it will often look like it works, but is still
wrong.

BTW, Normally the best way to create thread-safe counters in
J2SE5+ is AtomicInteger/AtomicLong, using getAndIncrement.

-Doug
=================

Doug Lea is the foremost expert in the field of concurrency.

Dino

one learn something new everyday...

adding volatile seems to be the best workaround working on all jdks (but there seem to be bug with volatiles on different platforms too)

/max

[quote="max"]one learn something new everyday...

adding volatile seems to be the best workaround working on all jdks (but there seem to be bug with volatiles on different platforms too)

/max[/quote]]

I know what you mean. Before reading Doug Lea's articles & books I used to believe that you can have simple getters/setters with no synchronization or volatile. You should check out the concurrent libraries (there's a backport to 1.4): it is impressive.

Dino

i know, but i did not realize the basis for the DLC issue affected counters ... but of course it does when thinking about it. (but again, would like to know which JVM's that fails on this)

[quote="max"]i know, but i did not realize the basis for the DLC issue affected counters ... but of course it does when thinking about it. (but again, would like to know which JVM's that fails on this)[/quote]

Sometimes it is hard to decifer between theoretical and reality. I know the VM spec allows for this but do VMs really do this.

Check out this comment from the commons collection:

http://jakarta.apache.org/commons/colle ... yList.html

It is the same issue. It doesn't tell which architectures is fails on.

Do you program correctly? or efficiently (with the remote possibility of wierd problems)? I know Doug Lea's response is...

It is amazing how 3 lines of code can be sooo complicated...


Dino