Synchronized on database when remove from lockSharedSessions, add test

h2/src/main/org/h2/engine/Session.java

@@ -923,14 +923,12 @@ public class Session extends SessionWithState {
             }
         }
         if (locks.size() > 0) {
-            synchronized (database) {
-                // don't use the enhanced for loop to save memory
-                for (int i = 0, size = locks.size(); i < size; i++) {
-                    Table t = locks.get(i);
-                    t.unlock(this);
-                }
-                locks.clear();
+            // don't use the enhanced for loop to save memory
+            for (int i = 0, size = locks.size(); i < size; i++) {
+                Table t = locks.get(i);
+                t.unlock(this);
             }
+            locks.clear();
         }
         savepoints = null;
         sessionStateChanged = true;

h2/src/main/org/h2/table/RegularTable.java

@@ -670,10 +670,10 @@ public class RegularTable extends TableBase {
             if (lockExclusiveSession == s) {
                 lockExclusiveSession = null;
             }
-            if (lockSharedSessions.size() > 0) {
-                lockSharedSessions.remove(s);
-            }
             synchronized (database) {
+                if (lockSharedSessions.size() > 0) {
+                    lockSharedSessions.remove(s);
+                }
                 if (!waitingSessions.isEmpty()) {
                     database.notifyAll();
                 }

h2/src/test/org/h2/test/TestAll.java

@@ -266,7 +266,7 @@ java org.h2.test.TestAll timer
     /**
      * Whether the MVStore storage is used.
      */
-    public final boolean mvStore = Constants.VERSION_MINOR >= 4;
+    public boolean mvStore = Constants.VERSION_MINOR >= 4;
 
     /**
      * If the test should run with many rows.
@@ -298,6 +298,11 @@ java org.h2.test.TestAll timer
      */
     public boolean mvcc = mvStore;
 
+    /**
+     * If the multi-threaded mode should be used.
+     */
+    public boolean multiThreaded;
+
     /**
      * The cipher to use (null for unencrypted).
      */

h2/src/test/org/h2/test/TestBase.java

@@ -280,6 +280,8 @@ public abstract class TestBase {
         if (config.mvStore) {
             url = addOption(url, "MV_STORE", "true");
             // url = addOption(url, "MVCC", "true");
+        } else {
+            url = addOption(url, "MV_STORE", "false");
         }
         if (!config.memory) {
             if (config.smallLog && admin) {
@@ -310,6 +312,9 @@ public abstract class TestBase {
         if (config.mvcc) {
             url = addOption(url, "MVCC", "TRUE");
         }
+        if (config.multiThreaded) {
+            url = addOption(url, "MULTI_THREADED", "TRUE");
+        }
         if (config.cacheType != null && admin) {
             url = addOption(url, "CACHE_TYPE", config.cacheType);
         }

h2/src/test/org/h2/test/synth/TestReleaseSelectLock.java

+package org.h2.test.synth;
+
+import org.h2.test.TestBase;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.Statement;
+import java.util.concurrent.CountDownLatch;
+
+/**
+ * Tests lock releasing for concurrent select statements
+ */
+public class TestReleaseSelectLock extends TestBase {
+
+    private static final String TEST_DB_NAME = "releaseSelectLock";
+
+    /**
+     * Run just this test.
+     *
+     * @param a ignored
+     */
+    public static void main(String... a) throws Exception {
+        TestBase.createCaller().init().test();
+    }
+
+    @Override
+    public void test() throws Exception {
+        config.mvStore = false;
+        config.mvcc = false;
+        config.multiThreaded = true;
+
+        deleteDb(TEST_DB_NAME);
+
+        Connection conn = getConnection(TEST_DB_NAME);
+        final Statement statement = conn.createStatement();
+        statement.execute("create table test(id int primary key)");
+
+        runConcurrentSelects();
+
+        // check that all locks have been released by dropping the test table
+        statement.execute("drop table test");
+
+        statement.close();
+        conn.close();
+        deleteDb(TEST_DB_NAME);
+    }
+
+    private void runConcurrentSelects() throws InterruptedException {
+        int tryCount = 500;
+        int threadsCount = getSize(2, 4);
+        for (int tryNumber = 0; tryNumber < tryCount; tryNumber++) {
+            final CountDownLatch allFinished = new CountDownLatch(threadsCount);
+
+            for (int i = 0; i < threadsCount; i++) {
+                new Thread(new Runnable() {
+                    @Override
+                    public void run() {
+                        try {
+                            Connection conn = getConnection(TEST_DB_NAME);
+                            PreparedStatement stmt = conn.prepareStatement("select id from test");
+                            ResultSet rs = stmt.executeQuery();
+                            while (rs.next()) {
+                                rs.getInt(1);
+                            }
+                            stmt.close();
+                            conn.close();
+                        } catch (Exception e) {
+                            throw new RuntimeException(e);
+                        } finally {
+                            allFinished.countDown();
+                        }
+                    }
+                }).start();
+            }
+
+            allFinished.await();
+        }
+    }
+}