You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by pa...@apache.org on 2013/08/14 15:03:26 UTC
[1/3] git commit: WICKET-5316: fixed synchronization problem in page
access
Updated Branches:
refs/heads/master bfd464965 -> e0fac9911
WICKET-5316: fixed synchronization problem in page access
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/53d9fae5
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/53d9fae5
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/53d9fae5
Branch: refs/heads/master
Commit: 53d9fae5d9886bce65c5e6f360a4294791bd762a
Parents: bfd4649
Author: Emond Papegaaij <em...@topicus.nl>
Authored: Wed Aug 14 14:18:49 2013 +0200
Committer: Emond Papegaaij <em...@topicus.nl>
Committed: Wed Aug 14 15:03:08 2013 +0200
----------------------------------------------------------------------
.../wicket/page/PageAccessSynchronizer.java | 70 +++++++++++++-------
1 file changed, 45 insertions(+), 25 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/wicket/blob/53d9fae5/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java b/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
index 721b584..da1f12b 100644
--- a/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
+++ b/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
@@ -114,23 +114,7 @@ public class PageAccessSynchronizer implements Serializable
long remaining = remaining(start, timeout);
if (remaining > 0)
{
- synchronized (previous)
- {
- if (isDebugEnabled)
- {
- logger.debug("{} waiting for lock to page {} for {}", new Object[] {
- thread.getName(), pageId, Duration.milliseconds(remaining) });
- }
- try
- {
- previous.wait(remaining);
- }
- catch (InterruptedException e)
- {
- // TODO better exception
- throw new RuntimeException(e);
- }
- }
+ previous.waitForRelease(remaining, isDebugEnabled);
}
}
}
@@ -212,14 +196,7 @@ public class PageAccessSynchronizer implements Serializable
lock.pageId);
}
// if any locks were removed notify threads waiting for a lock
- synchronized (lock)
- {
- if (isDebugEnabled)
- {
- logger.debug("'{}' notifying blocked threads", thread.getName());
- }
- lock.notifyAll();
- }
+ lock.markReleased(isDebugEnabled);
if (pageId != null)
{
// unlock just the page with the specified id
@@ -301,6 +278,8 @@ public class PageAccessSynchronizer implements Serializable
/** thread that owns the lock */
private final Thread thread;
+ private boolean released = false;
+
/**
* Constructor
*
@@ -328,5 +307,46 @@ public class PageAccessSynchronizer implements Serializable
{
return thread;
}
+
+ final synchronized void waitForRelease(long remaining, boolean isDebugEnabled)
+ {
+ if (released)
+ {
+ // the thread holding the lock released it before we were able to wait for the
+ // release
+ if (isDebugEnabled)
+ {
+ logger.debug(
+ "lock for page with id {} no longer locked by {}, falling through", pageId,
+ thread.getName());
+ }
+ return;
+ }
+
+ if (isDebugEnabled)
+ {
+ logger.debug("{} waiting for lock to page {} for {}",
+ new Object[] { thread.getName(), pageId, Duration.milliseconds(remaining) });
+ }
+ try
+ {
+ wait(remaining);
+ }
+ catch (InterruptedException e)
+ {
+ // TODO better exception
+ throw new RuntimeException(e);
+ }
+ }
+
+ final synchronized void markReleased(boolean isDebugEnabled)
+ {
+ if (isDebugEnabled)
+ {
+ logger.debug("'{}' notifying blocked threads", thread.getName());
+ }
+ released = true;
+ notifyAll();
+ }
}
}
[2/3] git commit: WICKET-5316: boolean flag should be volatile
Posted by pa...@apache.org.
WICKET-5316: boolean flag should be volatile
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/23bab6d4
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/23bab6d4
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/23bab6d4
Branch: refs/heads/master
Commit: 23bab6d4a538b03ab92d84a2f1982c510468a199
Parents: 53d9fae
Author: Emond Papegaaij <em...@topicus.nl>
Authored: Wed Aug 14 15:01:33 2013 +0200
Committer: Emond Papegaaij <em...@topicus.nl>
Committed: Wed Aug 14 15:03:13 2013 +0200
----------------------------------------------------------------------
.../main/java/org/apache/wicket/page/PageAccessSynchronizer.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/wicket/blob/23bab6d4/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java b/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
index da1f12b..478c4c3 100644
--- a/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
+++ b/wicket-core/src/main/java/org/apache/wicket/page/PageAccessSynchronizer.java
@@ -278,7 +278,7 @@ public class PageAccessSynchronizer implements Serializable
/** thread that owns the lock */
private final Thread thread;
- private boolean released = false;
+ private volatile boolean released = false;
/**
* Constructor
[3/3] git commit: WICKET-5316: testcase showing the problem with lock
releases
Posted by pa...@apache.org.
WICKET-5316: testcase showing the problem with lock releases
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/e0fac991
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/e0fac991
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/e0fac991
Branch: refs/heads/master
Commit: e0fac9911998c18b7a52d22b4b4253e97175be8b
Parents: 23bab6d
Author: Emond Papegaaij <em...@topicus.nl>
Authored: Wed Aug 14 15:01:43 2013 +0200
Committer: Emond Papegaaij <em...@topicus.nl>
Committed: Wed Aug 14 15:03:17 2013 +0200
----------------------------------------------------------------------
.../wicket/page/PageAccessSynchronizerTest.java | 58 ++++++++++++++++++++
1 file changed, 58 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/wicket/blob/e0fac991/wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java b/wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java
index c3770f4..b5cb403 100644
--- a/wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/page/PageAccessSynchronizerTest.java
@@ -17,7 +17,9 @@
package org.apache.wicket.page;
import java.util.Random;
+import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.wicket.MockPage;
@@ -327,4 +329,60 @@ public class PageAccessSynchronizerTest extends Assert
PageLock pageLock2 = locks.get(Integer.valueOf(pageId));
assertNotNull(pageLock2);
}
+
+ /**
+ * https://issues.apache.org/jira/browse/WICKET-5316
+ *
+ * @throws Exception
+ */
+ @Test
+ public void failToReleaseUnderLoad() throws Exception
+ {
+ final ConcurrentLinkedQueue<Exception> errors = new ConcurrentLinkedQueue<Exception>();
+ final long endTime = System.currentTimeMillis() + Duration.seconds(20).getMilliseconds();
+ final PageAccessSynchronizer sync = new PageAccessSynchronizer(Duration.seconds(10));
+ final CountDownLatch latch = new CountDownLatch(100);
+ for (int count = 0; count < 100; count++)
+ {
+ new Thread()
+ {
+ @Override
+ public void run()
+ {
+ try
+ {
+ while (System.currentTimeMillis() < endTime)
+ {
+ try
+ {
+ logger.debug(Thread.currentThread().getName() + " locking");
+ sync.lockPage(0);
+ Thread.sleep(1);
+ logger.debug(Thread.currentThread().getName() + " locked");
+ sync.unlockAllPages();
+ logger.debug(Thread.currentThread().getName() + " unlocked");
+ Thread.sleep(5);
+ }
+ catch (InterruptedException e)
+ {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+ catch (Exception e)
+ {
+ logger.error(e.getMessage(), e);
+ errors.add(e);
+ }
+ finally
+ {
+ latch.countDown();
+ }
+ }
+ }.start();
+ }
+ latch.await();
+ if (!errors.isEmpty())
+ throw errors.remove();
+ }
}