[jira] [Resolved] (JCR-4116) SharedItemStateManager.getNonVirtualItemState is over-synchronized

["Marcel Reutegger (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/JCR-4116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Marcel Reutegger resolved JCR-4116.
-----------------------------------
    Resolution: Invalid

bq. If one thread goes into the wait() method it locks all other threads out, which use the same SharedItemsStateManager instance, because of the synchronized (this).

This is not the case. The current thread calling {{wait()}} releases ownership of the monitor and allows other threads to enter the synchronized block.

See: https://docs.oracle.com/javase/8/docs/api/java/lang/Object.html#wait--

> SharedItemStateManager.getNonVirtualItemState is over-synchronized
> ------------------------------------------------------------------
>
>                 Key: JCR-4116
>                 URL: https://issues.apache.org/jira/browse/JCR-4116
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-core
>    Affects Versions: 2.10.5, 2.12.6, 2.15.0, 2.13.7, 2.14.0, 2.8.5
>            Reporter: Benjamin Papez
>
> With JCR-2813 and revision 1038201 bigger synchronization has been removed, but I think it is still too big.
> {code}
>         // Wait if another thread is already loading this item state
>         synchronized (this) {
>             while (currentlyLoading.contains(id)) {
>                 try {
>                     wait();
>                 } catch (InterruptedException e) {
>                     throw new ItemStateException(
>                             "Interrupted while waiting for " + id, e);
>                 }
>             }
>             state = cache.retrieve(id);
>             if (state != null) {
>                 return state;
>             }
>             // No other thread has loaded the item state, so we'll do it
>             currentlyLoading.add(id);
>         }
> {code}
> Looking at thread dumps I see that the reality does not match the comment {{Wait if another thread is already loading this item state}}, because threads have to wait even if they want a different item state. 
> If one thread goes into the wait() method it locks all other threads out, which use the same SharedItemsStateManager instance, because of the {{synchronized (this)}}. 
> I think that it would be better if {{currentlyLoading}} would be backed by a thread-safe {{CoincurrentHashMap}}, so {{synchronized (this)}} would not be necessary around {{currentlyLoading.contains}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)