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
>
>

Reply via email to