You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2014/09/01 05:09:41 UTC
[pool] time to cut 2.3
We have fixed a few bugs and made some enhancements since 2.2 and I
think we should cut 2.3. I will volunteer to RM. I will start
rolling RCs from trunk around the end of this week. I don't see any
of the current open bugs against 2.x as blockers. I may implement
the request in POOL-272 (or gladly accept a patch), but I think this
can wait for 2.4. Patches, beautification, late-breaking bugs
welcome.
Phil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [pool] time to cut 2.3
Posted by Jacopo Cappellato <ja...@gmail.com>.
Hi Gary, all,
I am sorry if I jump in in this conversation: I am a committer of another ASF project (I am actually the PMC chairman for Apache OFBiz) and my project is using dbcp2/pool2, so thank you for all the work you did and are doing.
I am reviewing the code in DefaultPooledObject and Gary is definitely right: the code is not thread safe; well, actually there is other code in the class that is not thread safe: the "state" field is properly accessed in a thread safe way but all the other fields are not. It is not enough to set them volatile because the class has some invariants that include more than one field.
Gary has mentioned one but for example also getLastUsedTime() doesn't seem thread safe.
As a side note, I also see room for a couple of minor code simplifications. Would you like me to send you a patch with some possible improvements?
Regards,
Jacopo
On Sep 27, 2014, at 3:04 PM, Gary Gregory <ga...@gmail.com> wrote:
> In org.apache.commons.pool2.impl.DefaultPooledObject.getActiveTimeMillis(),
> I see:
>
> // Take copies to avoid threading issues
> long rTime = lastReturnTime;
> long bTime = lastBorrowTime;
>
> But isn't is the case that the object could be returned and borrowed again
> between the two lines? Unlikely but possible?
>
> Gary
>
> On Sun, Aug 31, 2014 at 11:09 PM, Phil Steitz <ph...@gmail.com> wrote:
>
>> We have fixed a few bugs and made some enhancements since 2.2 and I
>> think we should cut 2.3. I will volunteer to RM. I will start
>> rolling RCs from trunk around the end of this week. I don't see any
>> of the current open bugs against 2.x as blockers. I may implement
>> the request in POOL-272 (or gladly accept a patch), but I think this
>> can wait for 2.4. Patches, beautification, late-breaking bugs
>> welcome.
>>
>> Phil
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [pool] time to cut 2.3
Posted by Phil Steitz <ph...@gmail.com>.
On 9/27/14 6:04 AM, Gary Gregory wrote:
> In org.apache.commons.pool2.impl.DefaultPooledObject.getActiveTimeMillis(),
> I see:
>
> // Take copies to avoid threading issues
> long rTime = lastReturnTime;
> long bTime = lastBorrowTime;
>
> But isn't is the case that the object could be returned and borrowed again
> between the two lines? Unlikely but possible?
I think the following code accounts for this race. If a
borrow-return were to sneak in between the two lines above, the
values would end up out of sequence, i.e., we would have rTime <
bTime. That would cause the check that follows to fail and rTime
would be replaced by System.currentTimeMillis() in the computation.
This would introduce a little error (I wonder if it would be better
to just re-read the - volatile - value of lastRturntime here), but
not much. Note also that this kind of thing could only happen if a
client were holding a reference to a DefaultPooledObject and calling
the (public) method directly. Internally, PooledObjectState
generally guards against this kind of race. The only internal use
is in returnObject, which uses state to prevent interleaving of this
kind. Factory methods that may use this method will in general be
similarly protected.
Phil
>
> Gary
>
> On Sun, Aug 31, 2014 at 11:09 PM, Phil Steitz <ph...@gmail.com> wrote:
>
>> We have fixed a few bugs and made some enhancements since 2.2 and I
>> think we should cut 2.3. I will volunteer to RM. I will start
>> rolling RCs from trunk around the end of this week. I don't see any
>> of the current open bugs against 2.x as blockers. I may implement
>> the request in POOL-272 (or gladly accept a patch), but I think this
>> can wait for 2.4. Patches, beautification, late-breaking bugs
>> welcome.
>>
>> Phil
>>
>> ---------------------------------------------------------------------
>> 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: [pool] time to cut 2.3
Posted by Gary Gregory <ga...@gmail.com>.
In org.apache.commons.pool2.impl.DefaultPooledObject.getActiveTimeMillis(),
I see:
// Take copies to avoid threading issues
long rTime = lastReturnTime;
long bTime = lastBorrowTime;
But isn't is the case that the object could be returned and borrowed again
between the two lines? Unlikely but possible?
Gary
On Sun, Aug 31, 2014 at 11:09 PM, Phil Steitz <ph...@gmail.com> wrote:
> We have fixed a few bugs and made some enhancements since 2.2 and I
> think we should cut 2.3. I will volunteer to RM. I will start
> rolling RCs from trunk around the end of this week. I don't see any
> of the current open bugs against 2.x as blockers. I may implement
> the request in POOL-272 (or gladly accept a patch), but I think this
> can wait for 2.4. Patches, beautification, late-breaking bugs
> welcome.
>
> Phil
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory