On 15/09/2015 18:35, Caldarale, Charles R wrote: >> From: Mark Thomas [mailto:ma...@apache.org] >> Subject: Re: svn commit: r1703194 - >> /tomcat/trunk/java/org/apache/tomcat/websocket/WsWebSocketContainer.java > >> The odds of a data race here are slim (to non-existent) > > That's definitely true; the questions arising here are more theoretical than > practical for this particular instance. However, they may be applicable to > other areas. > >> The use of volatile ensures that the current thread updating the >> field sees the value set by the previous thread to update it. > > Ok, I misunderstood the situation - didn't realize that something else is > preventing actual concurrent use. But since the whole issue is based on > multiple threads calling this method, what prevents the concurrency? (I've > seen threads on Linux interrupted and held up for seconds when unrelated page > table management is underway, allowing some very unexpected data sharing.) > If the multiple threads that arrive here sequentially are ordered by some > other mechanism (e.g., synchronization, context switch, termination), the > volatile still isn't needed.
What prevents the concurrency is that there is only ever one background thread. The only was I see this happening is: - T1 runs and expires all the current sessions. - The container unregisters itself from background processing since it has no sessions - It was the only thing registered for background processing so the T1 is terminated by the BackgroundProcessManager - A new WeBSoket session is opened - The container registers itself with the BackgroundProcessManager - A new background processing thread T2 is started - T2 runs RV-Predict indicated that there was a race. Was this a false positive? The overhead associated with using volatile is minimal but I'd be happy to drop it if it is not necessary. >> Some further comments in the code t explain the above are probably >> called for. > > That would indeed help. > >>> I am concerned that the iterator in the for loop of backgroundProcess() may >>> lose its >>> validity if another thread updates the map > >> The Javadoc for ConcurrentHashMap says this usage is safe. > > The only statement I can find in the Javadoc is this: > > "Similarly, Iterators, Spliterators and Enumerations return elements > reflecting the state of the hash table at some point at or since the creation > of the iterator/enumeration. They do not throw > ConcurrentModificationException." > > Not quite the reassurance I'm looking for that an iterator will actually be > useful (i.e., find all remaining entries) after the map has been mutated and > possibly reorganized by another thread. This is one spot where the C++ spec > is clearer (rather surprisingly). I was looking at keyset "they are guaranteed to traverse elements as they existed upon construction exactly once, and may (but are not guaranteed to) reflect any modifications subsequent to construction." Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
