From 4ca9dbd7418f4193daceccf00131f0c67dedcfe5 Mon Sep 17 00:00:00 2001 From: Heath Thomann Date: Wed, 14 Jan 2015 21:56:22 +0000 Subject: [PATCH] OPENJPA-2547: When two threads attempt to get a Pessimistic Lock, one thread gets a 'false' lock. Applied fix to trunk. git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@1651847 13f79535-47bb-0310-9956-ffa450edef68 --- .../jdbc/kernel/PessimisticLockManager.java | 8 +- .../kernel/PessimisticLockEntity.java | 47 +++++ .../kernel/TestPessimisticLockException.java | 186 ++++++++++++++++++ 3 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/kernel/PessimisticLockEntity.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/kernel/TestPessimisticLockException.java diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PessimisticLockManager.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PessimisticLockManager.java index 401129b1d..99dff9be5 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PessimisticLockManager.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PessimisticLockManager.java @@ -34,6 +34,7 @@ import org.apache.openjpa.jdbc.sql.DBDictionary; import org.apache.openjpa.jdbc.sql.SQLBuffer; import org.apache.openjpa.jdbc.sql.SQLFactory; import org.apache.openjpa.jdbc.sql.Select; +import org.apache.openjpa.kernel.LockLevels; import org.apache.openjpa.kernel.LockScopes; import org.apache.openjpa.kernel.MixedLockLevels; import org.apache.openjpa.kernel.OpenJPAStateManager; @@ -127,7 +128,12 @@ public class PessimisticLockManager Object id = sm.getObjectId(); ClassMapping mapping = (ClassMapping) sm.getMetaData(); - List sqls = sm.getLock() == null + //Code changed for OPENJPA-2449, code updated for OPENJPA-2547. OPENJPA-2547 added + //one check to determine if the lock is a value of LockLevels.LOCK_NONE. The first + //time a thread attempts to get a lock the lock will be null. If the thread can't + //get the lock because another thread holds it, the lock will be non-null and have + //a value of LockLevels.LOCK_NONE. + List sqls = (sm.getLock() == null || sm.getLock().equals(LockLevels.LOCK_NONE)) ? getLockRows(dict, id, mapping, fetch, _store.getSQLFactory()) : new ArrayList(); if (ctx.getFetchConfiguration().getLockScope() == LockScopes.LOCKSCOPE_EXTENDED) diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/kernel/PessimisticLockEntity.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/kernel/PessimisticLockEntity.java new file mode 100644 index 000000000..3b15dd28d --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/kernel/PessimisticLockEntity.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.openjpa.persistence.kernel; + +import javax.persistence.Entity; +import javax.persistence.Id; + +@Entity +public class PessimisticLockEntity { + + @Id + int id; + + String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/kernel/TestPessimisticLockException.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/kernel/TestPessimisticLockException.java new file mode 100644 index 000000000..fb5e830ea --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/kernel/TestPessimisticLockException.java @@ -0,0 +1,186 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.openjpa.persistence.kernel; + +import javax.persistence.EntityManager; +import javax.persistence.LockModeType; + +import org.apache.openjpa.jdbc.conf.JDBCConfiguration; +import org.apache.openjpa.jdbc.sql.DB2Dictionary; +import org.apache.openjpa.jdbc.sql.DBDictionary; +import org.apache.openjpa.jdbc.sql.OracleDictionary; +import org.apache.openjpa.persistence.OpenJPAEntityManager; +import org.apache.openjpa.persistence.OpenJPAPersistence; +import org.apache.openjpa.persistence.PessimisticLockException; +import org.apache.openjpa.persistence.test.SQLListenerTestCase; + +public class TestPessimisticLockException extends SQLListenerTestCase { + int pKey = 1; + static boolean doSleep = true; + + @Override + public void setUp() throws Exception { + super.setUp(PessimisticLockEntity.class); + } + + /* + * This test has only been verified on DB2 and Oracle. + */ + protected boolean skipTest() { + if (emf.getConfiguration() instanceof JDBCConfiguration) { + DBDictionary inst = ((JDBCConfiguration) emf.getConfiguration()).getDBDictionaryInstance(); + return !((inst instanceof DB2Dictionary) || (inst instanceof OracleDictionary)); + } + return true; + } + + /* + * This test will verify that two threads get a an appropriate pessimistic lock + * when they both request one at the same time. See JIRA OPENJPA-2547 for a more + * detailed description of this test. + */ + public void testPessimisticLockException() { + if (!skipTest()) { + + populate(); + + TestThread t1 = new TestThread(); + TestThread t2 = new TestThread(); + t1.start(); + t2.start(); + + while ((t1.isAlive() || t2.isAlive())) { + try { + Thread.sleep(5000); + } catch (InterruptedException e) { + } + } + + // One, and only one, thread should get a PersistenceException + if (t1.gotPLEx && t2.gotPLEx) { + fail("Both threads got a PersistenceLockException! " + + "Only one thread should have received a PersistenceLockException"); + } else if (t1.gotPLEx == false && t2.gotPLEx == false) { + fail("Neither thread got a PersistenceLockException! " + + "One thread should have received a PersistenceLockException"); + } else if (t1.count < 2 && t2.count < 2) { + fail("PersistenceLockException was received, but not the expected number of times! " + + "One thread should have received a PersistenceLockException at least twice."); + } + } + } + + private class TestThread extends Thread { + boolean gotPLEx = false; + int count = 0; + + public synchronized void run() { + OpenJPAEntityManager oem = OpenJPAPersistence.cast(emf.createEntityManager()); + oem.getTransaction().begin(); + + PessimisticLockEntity entity = oem.find(PessimisticLockEntity.class, pKey); + + boolean locked = false; + while (!locked) { + try { + oem.getFetchPlan().setLockTimeout(5000); + oem.lock(entity, LockModeType.PESSIMISTIC_READ); + locked = true; + } catch (PessimisticLockException ple) { + gotPLEx = true; + count++; + + try { + Thread.sleep(100); + } catch (final InterruptedException ie) { + } + oem.refresh(entity); + } catch (Throwable pe) { + pe.printStackTrace(); + fail("Caught an unexepected exception: " + pe); + } + + // Only one thread needs to sleep (don't care about synchronization of 'doSleep' at this + // point - if both threads happen to get here at the same time we will test for that later.) + if (doSleep) { + doSleep = false; + try { + // Sleep log enough to ensure the other thread times out at least two times. + Thread.sleep(15000); + } catch (final InterruptedException ie) { + } + } + + if (!oem.getTransaction().getRollbackOnly()) { + oem.getTransaction().commit(); + } + } + } + } + + /* + * This test verifies the correct number of SQL statements when using a pessimistic + * lock (See JIRA OPENJPA-2449). Prior to OPENJPA-2449, when requesting a pessimistic lock + * we would do a 'select' to get the entity, and turn around and do another select to get a + * Pessimistic lock...in other words, we'd generate (on DB2) these two SQL statements for the refresh: + * + * SELECT t0.name FROM PessimisticLockEntity t0 WHERE t0.id = ? + * SELECT t0.name FROM PessimisticLockEntity t0 WHERE t0.id = ? FOR READ ONLY WITH RR USE AND KEEP UPDATE LOCKS + * + * With the fix of OPENJPA-2449, we generate only one select, as follows: + * + * SELECT t0.name FROM PessimisticLockEntity t0 WHERE t0.id = ? FOR READ ONLY WITH RR USE AND KEEP UPDATE LOCKS + * + * Not only does this save an SQL, but more importantly, the few millisecond delay between the two selects + * won't occur.....in a multi-threaded env this delay could cause another thread to get the lock over this + * one when the refresh occurs at the same time. + */ + public void testSQLCount() { + if (!skipTest()) { + + populate(); + resetSQL(); + + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + + PessimisticLockEntity plEnt = em.find(PessimisticLockEntity.class, pKey); + + em.refresh(plEnt, LockModeType.PESSIMISTIC_WRITE); + + plEnt.setName("test"); + em.getTransaction().commit(); + em.close(); + assertEquals("There should only be 3 SQL statements", 3, getSQLCount()); + } + } + + public void populate() { + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + + PessimisticLockEntity pt = new PessimisticLockEntity(); + pt.setId(pKey); + + em.persist(pt); + + em.getTransaction().commit(); + em.close(); + } +}