On 11/03/2009, Dan Fabulich <d...@fabulich.com> wrote: > sebb wrote: > > > > Sorry, my last e-mail mentioned that QueryRunner was not thread-safe, > > but I did not provide a patch. > > > > Dang; I skimmed through other classes looking for unsafe members but > overlooked your main point. > > > > Or you could change the API further and insist that the DataSource is > provided at construction time; the variable could be then made final. > > > > This is the right fix. (In fact, in my haste I thought you'd already done > it!) We should change the API as necessary to make the class immutable. > > > > Two of the constructors would need to be removed as well. > > > > No, you could just set the DataSource to "null" in the constructor; its > Connection-less methods wouldn't work until you created a new object, but I > think that's fine. >
OK, I'd not noticed that the class was usable without the DataSource. Of course the alternative is to document the class as thread-unsafe... > > SqlNullCheckedResultSet has many variables that cannot be made final, but > the Javadoc does not claim it is thread-safe. > > > > It could be made thread-safe by synchronizing (or making volatile) all the > variables, but this might be too much overhead. Depends whether this class > is likely to be used from multiple threads or not. > > > > I would not attempt to synchronize this class, just leave it unsafe and let > users synchronize. We should document more explicitly that (unlike some > other classes in DbUtils) it's unsafe. I'm not sure that the class can be made thread-safe externally. It's easy enough to override the setters with synchronized versions, but the getters need to be synchronized as well to ensure that the data is published correctly. However the class stores the unsynchronized getters in the Map. So it would be necessary to override invoke() as well. If this is done, then the whole class has been overridden - one might as well say it has been rewritten. > -Dan > > > --------------------------------------------------------------------- > 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
