You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Sebb (JIRA)" <ji...@apache.org> on 2010/02/08 18:38:28 UTC

[jira] Created: (POOL-158) References to GenericKeyedObjectPool._minIdle are not always synchronized

References to GenericKeyedObjectPool._minIdle are not always synchronized
-------------------------------------------------------------------------

                 Key: POOL-158
                 URL: https://issues.apache.org/jira/browse/POOL-158
             Project: Commons Pool
          Issue Type: Bug
            Reporter: Sebb


References to GenericKeyedObjectPool._minIdle are not always synchronized:

The ensureMinIdle() and evict() methods both access the field outside the synch. block.
This is particularly bad as they are called from the timer thread.

The references could use getMinIdle(), which is synch., or it would probably be cheaper to move the references into the synch. blocks.

Better yet, can the field be made final? setMinIdle() is currently only called from Test cases and setConfig() which is not referenced AFAICT.


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (POOL-158) References to GenericKeyedObjectPool._minIdle are not always synchronized

Posted by "Sebb (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/POOL-158?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876110#action_12876110 ] 

Sebb commented on POOL-158:
---------------------------

I've removed the synch. from those methods

> References to GenericKeyedObjectPool._minIdle are not always synchronized
> -------------------------------------------------------------------------
>
>                 Key: POOL-158
>                 URL: https://issues.apache.org/jira/browse/POOL-158
>             Project: Commons Pool
>          Issue Type: Bug
>    Affects Versions: 1.2, 1.4, 1.5, 1.5.1, 1.5.2, 1.5.3, 1.5.4
>            Reporter: Sebb
>             Fix For: 1.5.5
>
>
> References to GenericKeyedObjectPool._minIdle are not always synchronized:
> The ensureMinIdle() and evict() methods both access the field outside the synch. block.
> This is particularly bad as they are called from the timer thread.
> The references could use getMinIdle(), which is synch., or it would probably be cheaper to move the references into the synch. blocks.
> Better yet, can the field be made final? setMinIdle() is currently only called from Test cases and setConfig() which is not referenced AFAICT.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (POOL-158) References to GenericKeyedObjectPool._minIdle are not always synchronized

Posted by "Sebb (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/POOL-158?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12831012#action_12831012 ] 

Sebb commented on POOL-158:
---------------------------

Note that GenericObjectPool is OK in this regard because it only accesses _minIdle via the synch. getter/setter after construction.

> References to GenericKeyedObjectPool._minIdle are not always synchronized
> -------------------------------------------------------------------------
>
>                 Key: POOL-158
>                 URL: https://issues.apache.org/jira/browse/POOL-158
>             Project: Commons Pool
>          Issue Type: Bug
>            Reporter: Sebb
>
> References to GenericKeyedObjectPool._minIdle are not always synchronized:
> The ensureMinIdle() and evict() methods both access the field outside the synch. block.
> This is particularly bad as they are called from the timer thread.
> The references could use getMinIdle(), which is synch., or it would probably be cheaper to move the references into the synch. blocks.
> Better yet, can the field be made final? setMinIdle() is currently only called from Test cases and setConfig() which is not referenced AFAICT.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (POOL-158) References to GenericKeyedObjectPool._minIdle are not always synchronized

Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/POOL-158?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Phil Steitz updated POOL-158:
-----------------------------

        Fix Version/s: 1.5.5
    Affects Version/s: 1.5.4
                       1.5.3
                       1.5.2
                       1.5.1
                       1.5
                       1.4
                       1.2

For correctness in evict, we probably technically need to move the read inside the synch block (otherwise in theory a write between lock acquisitions could invalidate the decision).  The ensureMinIdle read is similar, so moving both reads inside the synchs (carefully :) seems best.  Big +1 for deprecating the setter with aim to make final in 2.0.

> References to GenericKeyedObjectPool._minIdle are not always synchronized
> -------------------------------------------------------------------------
>
>                 Key: POOL-158
>                 URL: https://issues.apache.org/jira/browse/POOL-158
>             Project: Commons Pool
>          Issue Type: Bug
>    Affects Versions: 1.2, 1.4, 1.5, 1.5.1, 1.5.2, 1.5.3, 1.5.4
>            Reporter: Sebb
>             Fix For: 1.5.5
>
>
> References to GenericKeyedObjectPool._minIdle are not always synchronized:
> The ensureMinIdle() and evict() methods both access the field outside the synch. block.
> This is particularly bad as they are called from the timer thread.
> The references could use getMinIdle(), which is synch., or it would probably be cheaper to move the references into the synch. blocks.
> Better yet, can the field be made final? setMinIdle() is currently only called from Test cases and setConfig() which is not referenced AFAICT.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Resolved: (POOL-158) References to GenericKeyedObjectPool._minIdle are not always synchronized

Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/POOL-158?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Phil Steitz resolved POOL-158.
------------------------------

    Resolution: Fixed

After looking carefully at the code, I concluded that making the field volatile is a better solution.  For the evict() read, there is no other reason to acquire a pool lock in the initial check. 

> References to GenericKeyedObjectPool._minIdle are not always synchronized
> -------------------------------------------------------------------------
>
>                 Key: POOL-158
>                 URL: https://issues.apache.org/jira/browse/POOL-158
>             Project: Commons Pool
>          Issue Type: Bug
>    Affects Versions: 1.2, 1.4, 1.5, 1.5.1, 1.5.2, 1.5.3, 1.5.4
>            Reporter: Sebb
>             Fix For: 1.5.5
>
>
> References to GenericKeyedObjectPool._minIdle are not always synchronized:
> The ensureMinIdle() and evict() methods both access the field outside the synch. block.
> This is particularly bad as they are called from the timer thread.
> The references could use getMinIdle(), which is synch., or it would probably be cheaper to move the references into the synch. blocks.
> Better yet, can the field be made final? setMinIdle() is currently only called from Test cases and setConfig() which is not referenced AFAICT.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (POOL-158) References to GenericKeyedObjectPool._minIdle are not always synchronized

Posted by "Sebb (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/POOL-158?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876101#action_12876101 ] 

Sebb commented on POOL-158:
---------------------------

The access methods setMinIdle/getMinIdle are still synchronized. There's no need now that _minIdle is volatile.

> References to GenericKeyedObjectPool._minIdle are not always synchronized
> -------------------------------------------------------------------------
>
>                 Key: POOL-158
>                 URL: https://issues.apache.org/jira/browse/POOL-158
>             Project: Commons Pool
>          Issue Type: Bug
>    Affects Versions: 1.2, 1.4, 1.5, 1.5.1, 1.5.2, 1.5.3, 1.5.4
>            Reporter: Sebb
>             Fix For: 1.5.5
>
>
> References to GenericKeyedObjectPool._minIdle are not always synchronized:
> The ensureMinIdle() and evict() methods both access the field outside the synch. block.
> This is particularly bad as they are called from the timer thread.
> The references could use getMinIdle(), which is synch., or it would probably be cheaper to move the references into the synch. blocks.
> Better yet, can the field be made final? setMinIdle() is currently only called from Test cases and setConfig() which is not referenced AFAICT.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.