2013/1/26 Mark Thomas <ma...@apache.org>:
> kkoli...@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

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.

Best regards,
Konstantin Kolinko

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

Reply via email to