[
https://issues.apache.org/jira/browse/TAP5-2799?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17899948#comment-17899948
]
Ben Weidig commented on TAP5-2799:
----------------------------------
I haven't analyzed the surrounding code/context yet, but I still wanted to give
you my thoughts about it.
At first glance, it looks like an oversight to write-lock on {{getAttribute}},
as {{getAttributeNames}} only uses a read-lock.
However, {{getAttributeNames}} returns a list of immutable {{String}}, whereas
{{getAttribute}} returns an actual object that might get modified.
Whether it's the job of {{SessionImpl}} to prevent you from modifying the
returned object is debatable.
After looking into the looking code, I think your issue might stem from
something like the following scenario:
1. Thread A holds a read lock and attempts to upgrade to a write lock.
2. It releases the read lock.
3. Thread B acquires the write lock during this window.
4. Thread A is now blocked waiting for the write lock, but Thread B is blocked
waiting for Thread A's cleanup to finish.
The first step would be creating a proof-of-concept with mocks to validate the
hypothesis, and then, trying to improve locking.
> Thread Lockup when trying to read from a Session Object
> -------------------------------------------------------
>
> Key: TAP5-2799
> URL: https://issues.apache.org/jira/browse/TAP5-2799
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-http
> Affects Versions: 5.8.7
> Reporter: Alex Craddock
> Priority: Major
>
> I am not 100% sure how to recreate this but one of our servers froze up on
> the 443 port, when I looked into it and checked a thread dump I noticed 24
> threads all in this stack.
>
>
> {code:java}
> "http-nio-443-exec-25" #95 daemon prio=5 os_prio=0 cpu=6249001.07ms
> elapsed=138173.76s tid=0x00007fd974035800 nid=0x6288f waiting on condition
> [0x00007fd929edb000]
> java.lang.Thread.State: WAITING (parking)
> at jdk.internal.misc.Unsafe.park([email protected]/Native Method)
> - parking to wait for <0x000000047a350910> (a
> java.util.concurrent.locks.ReentrantReadWriteLock$NonfairSync)
> at
> java.util.concurrent.locks.LockSupport.park([email protected]/LockSupport.java:194)
> at
> java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt([email protected]/AbstractQueuedSynchronizer.java:885)
> at
> java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued([email protected]/AbstractQueuedSynchronizer.java:917)
> at
> java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire([email protected]/AbstractQueuedSynchronizer.java:1240)
> at
> java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock.lock([email protected]/ReentrantReadWriteLock.java:959)
> at
> org.apache.tapestry5.http.internal.services.TapestrySessionFactoryImpl$SessionLockImpl.acquireWriteLock(TapestrySessionFactoryImpl.java:110)
> at
> org.apache.tapestry5.http.internal.services.SessionImpl.getAttribute(SessionImpl.java:50)
> at
> org.apache.tapestry5.http.internal.services.ClusteredSessionImpl.getAttribute(ClusteredSessionImpl.java:56)
> {code}
>
>
> Which seemed a bit odd to me. When I looked into the code for
> "org.apache.tapestry5.http.internal.services.SessionImpl" I noticed this
>
> {code:java}
> public Object getAttribute(String name) {
> this.lock.acquireWriteLock();
> return this.session.getAttribute(name);
> }
> public List<String> getAttributeNames() {
> this.lock.acquireReadLock();
> return InternalUtils.toList(this.session.getAttributeNames());
> }
> public void setAttribute(String name, Object value) {
> this.lock.acquireWriteLock();
> this.session.setAttribute(name, value);
> } {code}
> I am not sure why, but I think it's a bug that when you are calling
> getAttribute, that it should only apply a read lock rather than a write lock.
> Not sure if this will solve my issue but I think this should be looked into.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
