[
https://issues.apache.org/jira/browse/JCR-4116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15892296#comment-15892296
]
Benjamin Papez commented on JCR-4116:
-------------------------------------
Ok, thanks for the clarification. But then the probiem is that you let
concurrent threads into a block, which operates on a non-thread-safe
collection.
> 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)
