David M. Lloyd wrote:
On 05/14/2008 11:57 AM, Emmanuel Lecharny wrote:
David M. Lloyd wrote:
On 05/14/2008 11:20 AM, Emmanuel Lecharny wrote:
What I can tell at least though is that the session configuration
properties provided by IoService should be volatile, because they
are accessed in a different thread (I/O processor) almost always.
Use synchronization, not volatile. it's too dangerous. When we are
sure that it works fine, we _may_ switch to volatile.
I don't understand this? Volatile isn't dangerous at all. It's
clearly defined and well-specified, and using it to make fields have
multi-thread visibility is a perfectly fine usage for them.
The problem is not that volatile is dangerous by itself. Its semantic
is really clear, assuming that you _know_ what you can and can't do.
This is what scared me : mis-used of volatile.
Before of using it, I would rather prefer that we expose the clear
benefits we will get from it (and please, bring the numbers with you
;) before using it all over the code.
Well, if you don't use it, then when someone calls a setter on the
property, the change may not be propagated to any other threads,
meaning that different threads could read different values for those
properties. If you do add volatile, then this doesn't happen - a
change in one thread is visible immediately thereafter from another
thread.
Synchronise the access to this property.
If you're suggesting that volatile isn't the appropriate solution to
this problem, I say that the burden is on you to explain why using the
correct tool for the job is not acceptable in this case, and what you
hope to solve (and be specific, FUD like "I'm afraid it will be
misused" isn't good enough) by using something more complex. We're
talking about fields that are accessed via simple getter and setter
methods here.
Synchronized getter and setter should be ok. Of course, you may have to
copy the property before sending it to the user. Using volatile does not
protect more than synchronize, so I see no reason to use it in this
case. You will have the very same problem in both case, except that
volatile is only meant to protect the visibility.
Again, you're saying "volatile is too dangerous", but you're not
really giving any specific reasons for this odd statement.
Ignorance is dangerous. People don't know about volatile. You can just
check by reading this thread. I don't expect people to magically know
everything about everything, simply because we want to (not to mention
that I'm a member of the 'people' group, of course ;). The fact that
volatile does not guarantee atomicity is enough a risk to justify my
assertion.
We are talking about minor improvement in this area. We have much
bigger issues and much more important thinsg to deal with in MINA 2.0.
Minor improvement? I thought we were talking about taking something
that doesn't work, and fixing it.
The 'performance' word was used in Trustin's answer to Dmitry mail. If
it does not work, then first synchronize the code in order to make it
thread safe. Volatile is just a weaker way to 'fix the code', not a
solution.
Dmitry first mail was about the problem he founds, and he suggested to
use volatile. I suggest we don't and that we synchronize first, until we
have fixed the problem.
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
