You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by ka...@apache.org on 2010/06/07 10:21:58 UTC
svn commit: r952138 - in /db/derby/code/trunk/java:
engine/org/apache/derby/impl/store/access/conglomerate/
testing/org/apache/derbyTesting/functionTests/tests/store/
Author: kahatlen
Date: Mon Jun 7 08:21:58 2010
New Revision: 952138
URL: http://svn.apache.org/viewvc?rev=952138&view=rev
Log:
DERBY-4676: NullPointerException on SELECT on INNER JOIN
Check whether the page is latched after waiting for a lock, and assume
that the row has been deleted if it is not latched.
Added:
db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/Derby4676Test.java (with props)
Modified:
db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/GenericConglomerateController.java
db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/OpenConglomerate.java
db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/_Suite.java
Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/GenericConglomerateController.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/GenericConglomerateController.java?rev=952138&r1=952137&r2=952138&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/GenericConglomerateController.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/GenericConglomerateController.java Mon Jun 7 08:21:58 2010
@@ -284,6 +284,14 @@ public abstract class GenericConglomerat
pos, (RowPosition) null, false, true);
}
+ if (pos.current_page == null)
+ {
+ // The page is not latched after locking the row. This happens if
+ // the row was deleted while we were waiting for the lock. Return
+ // false to indicate that the row is no longer valid. (DERBY-4676)
+ return false;
+ }
+
// Fetch the row.
// RESOLVE (STO061) - don't know whether the fetch is for update or not.
//
@@ -381,6 +389,14 @@ public abstract class GenericConglomerat
pos, (RowPosition) null, false, waitForLock);
}
+ if (pos.current_page == null)
+ {
+ // The page is not latched after locking the row. This happens if
+ // the row was deleted while we were waiting for the lock. Return
+ // false to indicate that the row is no longer valid. (DERBY-4676)
+ return false;
+ }
+
// Fetch the row.
// RESOLVE (STO061) - don't know whether the fetch is for update or not.
//
Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/OpenConglomerate.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/OpenConglomerate.java?rev=952138&r1=952137&r2=952138&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/OpenConglomerate.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/conglomerate/OpenConglomerate.java Mon Jun 7 08:21:58 2010
@@ -276,11 +276,25 @@ public abstract class OpenConglomerate
}
/**
+ * <p>
* Latch the page containing the current RowPosition.
+ * </p>
+ *
* <p>
* This implementation also automatically updates the RowPosition to
* point at the slot containing the current RowPosition. This slot
* value is only valid while the latch is held.
+ * </p>
+ *
+ * <p>
+ * If the row pointed to by {@code pos} does not exist (including the
+ * case where the page itself does not exist), the page will not be
+ * latched, and {@code pos.current_page} will be set to {@code null}.
+ * </p>
+ *
+ * @param pos the position to a row on the page that should be latched
+ * @return {@code true} if the page was successfully latched, or
+ * {@code false} otherwise
*
* @exception StandardException Standard exception policy.
**/
@@ -329,13 +343,18 @@ public abstract class OpenConglomerate
/**
+ * <p>
* Lock row at given row position for read.
+ * </p>
+ *
* <p>
* This routine requests a row lock NOWAIT on the row located at the given
* RowPosition. If the lock is granted NOWAIT the
* routine will return true. If the lock cannot be granted NOWAIT, then
* the routine will release the latch on "page" and then it will request
* a WAIT lock on the row.
+ * </p>
+ *
* <p>
* This implementation:
* Assumes latch held on current_page.
@@ -344,11 +363,22 @@ public abstract class OpenConglomerate
* If the current_rh field of RowPosition is null, it is assumed the we
* want to lock the indicated current_slot. Upon return current_rh will
* point to the record handle associated with current_slot.
+ * </p>
+ *
* <p>
* After waiting and getting the lock on the row, this routine will fix up
* RowPosition to point at the row locked. This means it will get the
* page latch again, and it will fix the current_slot to point at the
* waited for record handle - it may have moved while waiting on the lock.
+ * </p>
+ *
+ * <p>
+ * When this method returns, the page holding the row pointed to by the
+ * {@code RowLocation} is latched. This is however not the case if
+ * {@code moveForwardIfRowDisappears} is {@code false} and the row has
+ * disappeared. Then the latch will be released before the method returns,
+ * and {@code pos.current_page} will be set to {@code null}.
+ * </p>
*
* @param pos Position to lock.
* @param aux_pos If you have to give up latch to get lock, then also
@@ -356,8 +386,8 @@ public abstract class OpenConglomerate
* @param moveForwardIfRowDisappears
* If true, then this routine must handle the case where
* the row id we are waiting on disappears when the latch
- * is released. If false an exception will be thrown if
- * the row disappears.
+ * is released. If false, and the row disappears, the
+ * latch will be released again and false is returned.
* @param waitForLock
* if true wait for lock, if lock can't be granted NOWAIT,
* else if false, throw a lock timeout exception if the
@@ -470,6 +500,25 @@ public abstract class OpenConglomerate
return(lock_granted_with_latch_held);
}
+ /**
+ * <p>
+ * Lock the row at the given position for write.
+ * </p>
+ *
+ * <p>
+ * The page pointed to by the {@code RowPosition} is assumed to be latched
+ * when this method is called. If the lock cannot be obtained without
+ * waiting, the latch will be released and re-obtained when the lock has
+ * been acquired.
+ * </p>
+ *
+ * <p>
+ * If the latch was released while waiting for the lock, and the row does
+ * not exist after the lock is obtained, the latch will be released again
+ * before the method returns, and {@code pos.current_page} will be set to
+ * {@code null}.
+ * </p>
+ */
public boolean lockPositionForWrite(
RowPosition pos,
boolean forInsert,
Added: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/Derby4676Test.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/Derby4676Test.java?rev=952138&view=auto
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/Derby4676Test.java (added)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/Derby4676Test.java Mon Jun 7 08:21:58 2010
@@ -0,0 +1,153 @@
+/*
+ Class org.apache.derbyTesting.functionTests.tests.store.Derby4676Test
+
+ 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.derbyTesting.functionTests.tests.store;
+
+import java.sql.Connection;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.List;
+import junit.framework.Test;
+import org.apache.derbyTesting.junit.BaseJDBCTestCase;
+import org.apache.derbyTesting.junit.JDBC;
+import org.apache.derbyTesting.junit.TestConfiguration;
+
+/**
+ * Regression test for DERBY-4676.
+ */
+public class Derby4676Test extends BaseJDBCTestCase {
+ /** List of {@code HelperThread}s used in the test. */
+ private List threads;
+
+ public Derby4676Test(String name) {
+ super(name);
+ }
+
+ /** Create a suite of tests. */
+ public static Test suite() {
+ return TestConfiguration.defaultSuite(Derby4676Test.class);
+ }
+
+ /** Set up the test environment. */
+ protected void setUp() {
+ threads = new ArrayList();
+ }
+
+ /** Tear down the test environment. */
+ protected void tearDown() throws Exception {
+ super.tearDown();
+
+ List localThreads = threads;
+ threads = null;
+
+ // First, wait for all threads to terminate and close all connections.
+ for (int i = 0; i < localThreads.size(); i++) {
+ HelperThread t = (HelperThread) localThreads.get(i);
+ t.join();
+ Connection c = t.conn;
+ if (c != null && !c.isClosed()) {
+ c.rollback();
+ c.close();
+ }
+ }
+
+ // Then check if any of the helper threads failed.
+ for (int i = 0; i < localThreads.size(); i++) {
+ HelperThread t = (HelperThread) localThreads.get(i);
+ if (t.exception != null) {
+ fail("Helper thread failed", t.exception);
+ }
+ }
+ }
+
+ /**
+ * <p>
+ * Regression test case for DERBY-4676. Before the fix, fetching a row by
+ * its row location would sometimes fail with a NullPointerException if
+ * the row was deleted while the fetch operation was waiting for a lock.
+ * </p>
+ */
+ public void testConcurrentFetchAndDelete() throws Exception {
+ // Create a table to use in the test. Note that we need to have a
+ // non-covering index on the table so that the row location is fetched
+ // from the index and used to look up the row in the heap. If the
+ // index covers all the columns, we won't fetch the row location from
+ // it and the bug won't be reproduced.
+ Statement s = createStatement();
+ s.execute("create table t(x int, y int)");
+ s.execute("create index idx on t(x)");
+
+ // Create a thread that repeatedly inserts and deletes a row.
+ HelperThread thread = new HelperThread() {
+ void body(Connection conn) throws Exception {
+ Thread.sleep(1000); // Wait for the select loop to start so
+ // that the insert/delete loop doesn't
+ // complete before it has started.
+ Statement s = conn.createStatement();
+ for (int i = 0; i < 1000; i++) {
+ s.execute("insert into t values (1,2)");
+ s.execute("delete from t");
+ }
+ s.close();
+ }
+ };
+
+ startThread(thread);
+
+ // As long as the insert/delete thread is running, try to read the
+ // rows of the table using the index. This used to cause intermittent
+ // NullPointerExceptions.
+ while (thread.isAlive()) {
+ JDBC.assertDrainResults(s.executeQuery(
+ "select * from t --derby-properties index=idx"));
+ }
+ }
+
+ /**
+ * Helper class for running database operations in a separate thread and
+ * in a separate transaction.
+ */
+ private abstract class HelperThread extends Thread {
+ Exception exception;
+ Connection conn;
+
+ public void run() {
+ try {
+ conn = openDefaultConnection();
+ body(conn);
+ } catch (Exception ex) {
+ exception = ex;
+ }
+ }
+
+ abstract void body(Connection conn) throws Exception;
+ }
+
+ /**
+ * Start a helper thread and register it for automatic clean-up in
+ * {@link #tearDown()}.
+ *
+ * @param thread the helper thread to start
+ */
+ private void startThread(HelperThread thread) {
+ thread.start();
+ threads.add(thread);
+ }
+}
Propchange: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/Derby4676Test.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/_Suite.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/_Suite.java?rev=952138&r1=952137&r2=952138&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/_Suite.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/store/_Suite.java Mon Jun 7 08:21:58 2010
@@ -57,6 +57,7 @@ public class _Suite extends BaseTestCase
suite.addTest(StreamingColumnTest.suite());
suite.addTest(Derby3625Test.suite());
suite.addTest(Derby151Test.suite());
+ suite.addTest(Derby4676Test.suite());
suite.addTest(BootLockTest.suite());
suite.addTest(PositionedStoreStreamTest.suite());
suite.addTest(OSReadOnlyTest.suite());