tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <>
Subject Re: svn commit: r1438747 - /tomcat/trunk/test/org/apache/tomcat/websocket/
Date Mon, 28 Jan 2013 11:27:43 GMT
On 26/01/2013 12:30, Konstantin Kolinko wrote:
> 2013/1/26 Mark Thomas <>:
>> wrote:
>>> Author: kkolinko
>>> Date: Fri Jan 25 22:34:57 2013
>>> New Revision: 1438747
>>> URL:
>>> 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

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

> 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.


To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message