[jira] [Commented] (TAP5-2820) ConcurrentBarrier hangs under heavy contention

["ASF subversion and git services (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TAP5-2820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18067266#comment-18067266 ] 

ASF subversion and git services commented on TAP5-2820:
-------------------------------------------------------

Commit 1be8d9944c9b69beb0bf3eacd7bb8af96909d390 in tapestry-5's branch refs/heads/master from Ben Weidig
[ https://gitbox.apache.org/repos/asf?p=tapestry-5.git;h=1be8d9944 ]

TAP5-2820: ConcurrentBarrier remove synchronized blocks

Under heavy contention, the synchronized blocks introduced a global
monitor on a ThreadLocal that can lead to livelock/starvation.

ReentrantWriteLock already supports local thread-based locking, so
relying on that is the better approach.

Furthermore, InterrupedExceptions were silently swallowed without
setting the flag, which is now no longer the case.

The existing tests were converted from Spock to Java.


> ConcurrentBarrier hangs under heavy contention
> ----------------------------------------------
>
>                 Key: TAP5-2820
>                 URL: https://issues.apache.org/jira/browse/TAP5-2820
>             Project: Tapestry 5
>          Issue Type: Bug
>          Components: tapestry-ioc
>            Reporter: Ben Weidig
>            Assignee: Ben Weidig
>            Priority: Major
>
> h3. Summary
> Under heavy contention, like the 1000 threads test in {{ConcurrentBarrierSpec}}, we experience intermittent infinite hangs due to starvation.
> h3. Why it happens
> All accesses to the {{threadHasReadLock}} are wrapped in synchronized(threadHasReadLock) blocks.
> These blocks introduce a global monitor (the ThreadLocal instance itself) that every thread must acquire alongside the read/write locks.
> However, the variable is already a {{ThreadLocal}}, making the unnecessary, as {{get()}}, {{set()}}, and {{remove()}} are inherently per-thread and require no external synchronization.
> Most likely, the heavy contention of the tests make OS context switching slow, so timeouts might happen.
> But any {{InterruptedException}} is simply swallowed without setting the interrupted flag.
> h3. Proposed Fix
> Removing the ThreadLocal and the synchronized blocks and rely on {{ReentrantReadWriteLock#getReadHoldCount()}} and {{ReentrantReadWriteLock#isWriteLockedByCurrentThread()}} instead.
> Also, restore the thread interrupt status instead of swalling it.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)