[ 
https://issues.apache.org/jira/browse/POOL-279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14165066#comment-14165066
 ] 

Sebb commented on POOL-279:
---------------------------

@[~jacopoc] Just noticed some odd code in the unit tests. The Runnable 
allocateAndDeallocateTask starts by performing the same test as getIdleTimeTask 
- why is that needed? Also, getIdleTimeTask exits the loop as soon as the first 
error is found, however that is not the case for allocateAndDeallocateTask. Why 
is that?

It also occurs to me that it might be simpler to use the following code:

{code}
final long elapsed = System.currentTimeMillis() - lastReturnTime;
// elapsed may be negative if:
// - another thread updates lastReturnTime during the calculation window
// - System.currentTimeMillis() is not monotonic (e.g. system time is set back)
return elapsed >= 0 ? elapsed : 0;
{code}

> Thread concurrency issue in DefaultPooledObject.getIdleTimeMillis()
> -------------------------------------------------------------------
>
>                 Key: POOL-279
>                 URL: https://issues.apache.org/jira/browse/POOL-279
>             Project: Commons Pool
>          Issue Type: Bug
>    Affects Versions: 2.2
>            Reporter: Jacopo Cappellato
>            Priority: Minor
>         Attachments: POOL-279-unit-test.patch, POOL-279.patch, 
> POOL-279.patch, POOL-279.patch, POOL-279.patch
>
>
> Under unlucky thread concurrency the getIdleTimeMillis() method of 
> DefaultPooledObject can return a negative value.
> I have attached a Junit test that fails most of the times and a simple fix, 
> that doesn't use synchronization: with this fix the Junit test always succeed.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to