You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Dan Fabulich <da...@fabulich.com> on 2009/03/11 06:33:43 UTC
[VOTE] Release of DbUtils 1.2 RC2
My second attempt at releasing a commons project; please be gentle. :-)
RC2 includes sebb's patches that make numerous instance variables
immutable.
NOTE: No one has yet explicitly said on-list that they have tested DbUtils
1.2 RC1 or RC2 with a real database. We should not release it until
somebody tries it out with a real live Oracle database, as described
below.
Compatibility warnings:
* We upgraded the JVM dependency from JDK 1.3 to JDK 1.4 (DBUTILS-31)
* Users who may have extended BeanListHandler.handleRow will find that
this method no longer exists (is no longer called) in DbUtils 1.2
(DBUTILS-37)
* Users who may have extended KeyedHandler will find that its protected
members are now final (to guarantee thread safety). (DBUTILS-51)
PLEASE TEST THIS RELEASE WITH A REAL DATABASE!
Although this project has reasonable unit tests, it has no integration
tests with any actual databases; it is quite possible that the fix for
DBUTILS-31 has broken something on Oracle, MS SQL Server, Derby, or your
favorite database.
To verify DBUTILS-31, use QueryRunner to put a null value in a field, e.g.
with QueryRunner.update. Ideally it would be good to verify putting nulls
in fields of various types: char, varchar, int, boolean, date, etc.
--
Tag:
https://svn.apache.org/repos/asf/commons/proper/dbutils/tags/DBUTILS_1_2
Site:
http://people.apache.org/builds/commons/dbutils/1.2/RC2/site/index.html
Binaries:
http://people.apache.org/builds/commons/dbutils/1.2/RC2/staged/commons-dbutils/commons-dbutils/1.2/
[ ] +1 release it
[ ] +0 go ahead I don't care
[ ] -1 no, do not release it because
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC2
Posted by Liam Coughlin <ls...@gmail.com>.
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 <se...@gmail.com> wrote:
> On 15/03/2009, Dan Fabulich <da...@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-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [VOTE] Release of DbUtils 1.2 RC2
Posted by sebb <se...@gmail.com>.
On 15/03/2009, Dan Fabulich <da...@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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC2
Posted by Dan Fabulich <da...@fabulich.com>.
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.
>> 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*
-Dan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC2
Posted by sebb <se...@gmail.com>.
On 11/03/2009, Dan Fabulich <da...@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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC2
Posted by Dan Fabulich <da...@fabulich.com>.
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.
> 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.
-Dan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC2
Posted by sebb <se...@gmail.com>.
Sorry, my last e-mail mentioned that QueryRunner was not thread-safe,
but I did not provide a patch.
The DataSource variable is protected.
To allow multi-threaded access, either the variable has to be made
volatile, or it has to be made private (i.e. an API change) and the
accessors need to be synchronised, and the class has to use the
accessors.
Or you could change the API further and insist that the DataSource is
provided at construction time; the variable could be then made final.
Two of the constructors would need to be removed as well.
There's another potential problem with QueryRunner, in that the
constructor calls an overrideable public method. This could cause
problems if the class is extended, as the subclass may not be fully
constructed when the method is invoked.
But an easy fix is to add a private set method used by both the
constructor and the public method.
I did not check the wrappers previously.
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.
On 11/03/2009, Dan Fabulich <da...@fabulich.com> wrote:
>
> My second attempt at releasing a commons project; please be gentle. :-)
>
> RC2 includes sebb's patches that make numerous instance variables
> immutable.
>
> NOTE: No one has yet explicitly said on-list that they have tested DbUtils
> 1.2 RC1 or RC2 with a real database. We should not release it until
> somebody tries it out with a real live Oracle database, as described below.
>
> Compatibility warnings:
>
> * We upgraded the JVM dependency from JDK 1.3 to JDK 1.4 (DBUTILS-31)
> * Users who may have extended BeanListHandler.handleRow will find that this
> method no longer exists (is no longer called) in DbUtils 1.2 (DBUTILS-37)
> * Users who may have extended KeyedHandler will find that its protected
> members are now final (to guarantee thread safety). (DBUTILS-51)
>
> PLEASE TEST THIS RELEASE WITH A REAL DATABASE!
>
> Although this project has reasonable unit tests, it has no integration
> tests with any actual databases; it is quite possible that the fix for
> DBUTILS-31 has broken something on Oracle, MS SQL Server, Derby, or your
> favorite database.
>
> To verify DBUTILS-31, use QueryRunner to put a null value in a field, e.g.
> with QueryRunner.update. Ideally it would be good to verify putting nulls
> in fields of various types: char, varchar, int, boolean, date, etc.
>
> --
>
> Tag:
>
> https://svn.apache.org/repos/asf/commons/proper/dbutils/tags/DBUTILS_1_2
>
> Site:
>
> http://people.apache.org/builds/commons/dbutils/1.2/RC2/site/index.html
>
> Binaries:
>
> http://people.apache.org/builds/commons/dbutils/1.2/RC2/staged/commons-dbutils/commons-dbutils/1.2/
>
> [ ] +1 release it
> [ ] +0 go ahead I don't care
> [ ] -1 no, do not release it because
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release of DbUtils 1.2 RC2
Posted by Jörg Schaible <jo...@gmx.de>.
+1, all tests pass now also on IBM JDK 6.
Dan Fabulich wrote:
>
> My second attempt at releasing a commons project; please be gentle. :-)
>
> RC2 includes sebb's patches that make numerous instance variables
> immutable.
>
> NOTE: No one has yet explicitly said on-list that they have tested DbUtils
> 1.2 RC1 or RC2 with a real database. We should not release it until
> somebody tries it out with a real live Oracle database, as described
> below.
>
> Compatibility warnings:
>
> * We upgraded the JVM dependency from JDK 1.3 to JDK 1.4 (DBUTILS-31)
> * Users who may have extended BeanListHandler.handleRow will find that
> this method no longer exists (is no longer called) in DbUtils 1.2
> (DBUTILS-37)
> * Users who may have extended KeyedHandler will find that its protected
> members are now final (to guarantee thread safety). (DBUTILS-51)
>
> PLEASE TEST THIS RELEASE WITH A REAL DATABASE!
>
> Although this project has reasonable unit tests, it has no integration
> tests with any actual databases; it is quite possible that the fix for
> DBUTILS-31 has broken something on Oracle, MS SQL Server, Derby, or your
> favorite database.
>
> To verify DBUTILS-31, use QueryRunner to put a null value in a field, e.g.
> with QueryRunner.update. Ideally it would be good to verify putting nulls
> in fields of various types: char, varchar, int, boolean, date, etc.
>
> --
>
> Tag:
>
> https://svn.apache.org/repos/asf/commons/proper/dbutils/tags/DBUTILS_1_2
>
> Site:
>
> http://people.apache.org/builds/commons/dbutils/1.2/RC2/site/index.html
>
> Binaries:
>
>
http://people.apache.org/builds/commons/dbutils/1.2/RC2/staged/commons-dbutils/commons-dbutils/1.2/
>
> [ ] +1 release it
> [ ] +0 go ahead I don't care
> [ ] -1 no, do not release it because
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org