Hello,
Brian Goetz [1] writes:
"You can use volatile variables instead of locks only under a
restricted set of circumstances. Both of the following criteria must
be met for volatile variables to provide the desired thread-safety:
* Writes to the variable do not depend on its current value.
* The variable does not participate in invariants with other variables.
...
However, if you can arrange that the value is only ever written from a
single thread, then you can ignore the first condition.)"
But at line 445 of BaseIoSession.java I see:
idleCountForRead++;
(in a public method called increaseIdleCount so I don't think we can
guarantee that only one thread will ever call it)
So it's pretty obvious to me that volatile IS NOT good enough if you
want a thread-safe solution.
[1] http://www.ibm.com/developerworks/java/library/j-jtp06197.html
[2]
http://mina.apache.org/report/1.1/xref/org/apache/mina/common/support/BaseIoSession.html#445
regards,
Maarten
On Wed, May 14, 2008 at 7:44 PM, David M. Lloyd <[EMAIL PROTECTED]> wrote:
> On 05/14/2008 12:22 PM, Emmanuel Lecharny wrote:
>>
>> 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.
>
> Why? Synchronization is not needed.
>
>> 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.
>
> The values don't need more protection.
>
>> You will have the very same problem in both case, except that volatile is
>> only meant to protect the visibility.
>
> Visibility is the only problem here that needs to be solved.
>
>
>> The fact that volatile does not guarantee atomicity is enough a risk to
>> justify my assertion.
>
> I don't understand. Atomicity is not a requirement here! The only
> requirement is that when the values are changed, the changes are immediately
> visible to other threads. Volatile solves this issue, and in fact was
> designed for this. What additional semantics does synchronization provide
> that are not solved by volatile?
>
>> 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.
>
> This makes no sense. Fear is not a reason to use or not use something. The
> problem is the lack of visibility, and the solution is volatile.
>
> - DML
>
