tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1438747 - /tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
Date Mon, 28 Jan 2013 11:27:43 GMT
On 26/01/2013 12:30, Konstantin Kolinko wrote:
> 2013/1/26 Mark Thomas <markt@apache.org>:
>> kkolinko@apache.org wrote:
>>
>>> Author: kkolinko
>>> Date: Fri Jan 25 22:34:57 2013
>>> New Revision: 1438747
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1438747&view=rev
>>> Log:
>>> Make the messages list synchronized as a whole, instead of just using a
>>> volatile reference to it.
>>> I am still observing random failures with TestWsWebSocketContainer, so
>>> an issue is not here.
>>
>> So why make the change?
>>
> 
> Because of
> http://svn.apache.org/viewvc?view=revision&revision=1437930

OK. The log message seemed to suggest the change was pointless. Hence my
question.

> If you just need to propagate the "messages" reference across threads,
> then I think r1437930 should have been more simple: just marking the
> field as "final".
> If you are protecting access to inner structures of ArrayList class, I
> think that marking the reference as volatile is not enough and that
> using a synchronized list is more adequate.
> 
> I think "volatile" modifier applies to accessing the filed itself, and
> has no effect when you use the value returned by getMessages() or
> store it in a local variable.
> 
> Well, I do not see where a concurrency can come from here, so mere
> "final" should be enough. The latch did count down (otherwise
> assertTrue(latchResult) detected a failure) so either a message was
> received or TesterEndpoint.onClose() or TesterEndpoint.onError() was
> called. The latch should provide synchronization between threads.

Having just read up on "happens-before" I do not believe the list needs
to be synchronised. I believe the root cause of the problem I was trying
to solve was fixed in r1437930 when I switched the order of adding the
message to the list and counting down the latch.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message