db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kahat...@apache.org
Subject svn commit: r952138 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/store/access/conglomerate/ testing/org/apache/derbyTesting/functionTests/tests/store/
Date Mon, 07 Jun 2010 08:21:58 GMT
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());



Mime
View raw message