"Knut Anders Hatlen (JIRA)" <[EMAIL PROTECTED]> writes:

>     [ 
> https://issues.apache.org/jira/browse/DERBY-2114?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12481937
>  ] 
>
> Knut Anders Hatlen commented on DERBY-2114:
> -------------------------------------------
>
> Thank you for addressing my comments! There is one thing I missed in the 
> previous review:
>
> +     public boolean containsKey(Object k) {
> +             synchronized(cache_) {
>
> +     public Collection values() {
> +             synchronized (cache_) {
>
> I believe these two methods should have been synchronized on this, not on 
> cache_. 

Agreed. All manipulation of cache_ synchronizes on this, so testing
for keys probably should too. The question is if you want to declare the
method as synchronized, or not. Perhaps it will not be necessary to synchronize
on this if/when the Clock implementation is changed to allow more concurrency?

-- 
dt

Reply via email to