On 15/09/2015 18:28, Felix Schumacher wrote: > Hi Mark, > > Am 15.09.2015 um 14:45 schrieb ma...@apache.org: >> Author: markt >> Date: Tue Sep 15 12:45:48 2015 >> New Revision: 1703177 >> >> URL: http://svn.apache.org/r1703177 >> Log: >> Use single object (membersLock) for all locking >> Make members volatile so single reads are safe > I am not sure, if it is enough to make members volatile. I think the > read/write guarantees are only valid if you change the variable > (reference) itself, not if you modify the contents of the array. > > This might be a problem, if Arrays.sort sorts the array in place.
Correct. I took the comments in the previous code at face value and assumed that Arrays.sort() would be applied before setting members. A quick look shows that isn't the case. I'll do a quick review of the code for similar issues and get them fixed. > Another (old) problem might be that getMembers returns the intern array > reference, which could be changed by the client and thus messing with > the inner state of the class. And getMember(Member) could probably throw > an IndexOutOfBoundException, when members would be decreased while the > method iterates over it (probably unlikely). I think we can live with that. Fixing it would require defensive copies or changing the API to use, for example, a read-only list. In think either of those options would be too expensive. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org