jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Reutegger (JIRA)" <j...@apache.org>
Subject [jira] [Resolved] (JCR-4116) SharedItemStateManager.getNonVirtualItemState is over-synchronized
Date Thu, 02 Mar 2017 10:43:45 GMT

     [ 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)

Mime
View raw message