On Dec 13, 2011, at 15:50, sebb <seb...@gmail.com> wrote: > 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.
+1 Gary > > 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
