I'd suggest marking it volatile or making it an immutable property. the overhead incurred from enforcing thread safety i think is a bit much for the specific purpose of the QueryRunner -- in all the instances that you mentioned -- it's the datasource that should be dispatched to threads not the queryRunner anyways.
On Sun, Mar 15, 2009 at 2:52 PM, sebb <seb...@gmail.com> wrote: > On 15/03/2009, Dan Fabulich <d...@fabulich.com> wrote: > > sebb wrote: > > > > > > > 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... > > > > > > > I would guess that the reason we've never seen a bug filed on this issue > is > > that nobody uses setDataSource after the class is created. For these > users, > > QueryRunner is thread-safe. I think just formalizing that state is best. > > If you mean nobody uses setDataSource at all, then I agree that cannot > affect thread-safety. > > However, if anyone uses setDataSource (which has to be after creation) > and passes the instance to another thread, then the receiving thread > may not see the updated value for the ds variable, i.e. it would not > be thread-safe. > > > > > > > > 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. > > > > > > > I didn't mean that users would synchronize externally by > > extending/overriding, but just by synchronizing access to an instance > > member, or just not sharing them across threads. *shrug* > > For a mutable instance field to be thread-safe, both writes and reads > need to be synchronized (or volatile). It's not enough to synch. just > the writes, and readers and writers must all use the same lock. > > > > > -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 > >
