On 13 December 2011 20:25, Mark Thomas <ma...@apache.org> wrote: > On 13/12/2011 19:12, sebb wrote: >> I've added some Javadoc to classes to indicate which ones are supposed >> to be thread-safe and which are not. >> >> AFAICT, there remain 2 classes to be dealt with, ie >> >> GenericKeyedObjectPool (GKOP) >> and >> GenericObjectPool (GOP) >> >> GKOP is not currently thread-safe, as there are several mutable >> variables that aren't always accessed using synch. >> Adding volatile to these would fix the safe publication issue, but >> might not fix all thread-safety issues. >> This depends on how exactly the code uses the variables. >> If the code relies on the variable not changing between references, >> then the code is not thread-safe. >> >> GOP is not thread-safe either, as the field evictionIterator is not volatile. >> Adding volatile would ensure safe publication, but the field is >> accessed multiple times - and may be reset to null. >> As far as I can tell, the evict() method is not thread-safe because of >> the way it changes the field. >> I suspect the code will need to use some synchronisation. Of course >> the same applies to GKOP. >> >> Making all the configuration items final and removing the setters - as >> discussed previously - will make it much easier to make the classes >> thread-safe. >> I think that's the next stage. > > -1. That makes use with DBCP and JNDI much, much harder.
Don't they know in advance what options are needed? Can't they just apply the changes to the config classes? Maybe we need an interface for DBCP and JNDI that allows the underlying classes to be immutable as far as possible. Are all of the config items needed for DBCP and JNDI? If not, which ones could be write-once? > Lets fix the thread safety issues, being careful to avoid adding syncs > unless we absolutely have to. We should also be careful not to expose more API than we absolutely have to. Regarding which - does the evict() method have to be public? If it were only called by the evictor thread, that might simplify the threading requirements. Does it make sense for the evictor code to be run by several threads in parallel? If it is required to be public, it would be a lot easier to make it synch. > Mark > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
