Make TestMultiThreadedKernel to run with MVStore:

fix ABBA deadlock in MVStore between database and meta table lock
fix premature release of database object ids in MVStore

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

@@ -102,6 +102,7 @@ public class Database implements DataHandler {
     private static final ThreadLocal<Session> META_LOCK_DEBUGGING;
     private static final ThreadLocal<Database> META_LOCK_DEBUGGING_DB;
     private static final ThreadLocal<Throwable> META_LOCK_DEBUGGING_STACK;
+    private static final Session[] EMPTY_SESSION_ARRAY = new Session[0];
 
     static {
         boolean a = false;
@@ -179,7 +180,7 @@ public class Database implements DataHandler {
     private int allowLiterals = Constants.ALLOW_LITERALS_ALL;
 
     private int powerOffCount = initialPowerOffCount;
-    private int closeDelay;
+    private volatile int closeDelay;
     private DelayedDatabaseCloser delayedCloser;
     private volatile boolean closing;
     private boolean ignoreCase;
@@ -786,10 +787,10 @@ public class Database implements DataHandler {
         data.create = create;
         data.isHidden = true;
         data.session = systemSession;
+        starting = true;
         meta = mainSchema.createTable(data);
         handleUpgradeIssues();
         IndexColumn[] pkCols = IndexColumn.wrap(new Column[] { columnId });
-        starting = true;
         metaIdIndex = meta.addIndex(systemSession, "SYS_ID",
                 0, pkCols, IndexType.createPrimaryKey(
                 false, false), true, null);
@@ -953,12 +954,15 @@ public class Database implements DataHandler {
         }
     }
 
-    private synchronized void addMeta(Session session, DbObject obj) {
+    private void addMeta(Session session, DbObject obj) {
+        assert Thread.holdsLock(this);
         int id = obj.getId();
         if (id > 0 && !starting && !obj.isTemporary()) {
             Row r = meta.getTemplateRow();
             MetaRecord.populateRowFromDBObject(obj, r);
-            objectIds.set(id);
+            synchronized (objectIds) {
+                objectIds.set(id);
+            }
             if (SysProperties.CHECK) {
                 verifyMetaLocked(session);
             }
@@ -1049,7 +1053,7 @@ public class Database implements DataHandler {
      * @param session the session
      * @param id the id of the object to remove
      */
-    public synchronized void removeMeta(Session session, int id) {
+    public void removeMeta(Session session, int id) {
         if (id > 0 && !starting) {
             SearchRow r = meta.getTemplateSimpleRow(false);
             r.setValue(0, ValueInt.get(id));
@@ -1075,7 +1079,25 @@ public class Database implements DataHandler {
                     unlockMeta(session);
                 }
             }
-            objectIds.clear(id);
+            if (isMVStore()) {
+                // release of the object id has to be postponed until the end of the transaction,
+                // otherwise it might be re-used prematurely, and it would make
+                // rollback impossible or lead to MVMaps name collision,
+                // so until then ids are accumulated within session
+                session.releaseDatabaseObjectId(id);
+            } else {
+                // but PageStore, on the other hand, for reasons unknown to me,
+                // requires immediate id release
+                synchronized (this) {
+                    objectIds.clear(id);
+                }
+            }
+        }
+    }
+
+    void releaseDatabaseObjectIds(BitSet idsToRelease) {
+        synchronized (objectIds) {
+            objectIds.andNot(idsToRelease);
         }
     }
 
@@ -1307,7 +1329,7 @@ public class Database implements DataHandler {
     }
 
     private synchronized void closeAllSessionsException(Session except) {
-        Session[] all = userSessions.toArray(new Session[userSessions.size()]);
+        Session[] all = userSessions.toArray(EMPTY_SESSION_ARRAY);
         for (Session s : all) {
             if (s != except) {
                 try {
@@ -1573,9 +1595,13 @@ public class Database implements DataHandler {
      *
      * @return the id
      */
-    public synchronized int allocateObjectId() {
-        int i = objectIds.nextClearBit(0);
-        objectIds.set(i);
+    public int allocateObjectId() {
+        Object lock = isMVStore() ? objectIds : this;
+        int i;
+        synchronized (lock) {
+            i = objectIds.nextClearBit(0);
+            objectIds.set(i);
+        }
         return i;
     }
 
@@ -1763,18 +1789,18 @@ public class Database implements DataHandler {
      */
     public void updateMeta(Session session, DbObject obj) {
         if (isMVStore()) {
-            synchronized (this) {
-                int id = obj.getId();
-                if (id > 0) {
-                    if (!starting && !obj.isTemporary()) {
-                        Row newRow = meta.getTemplateRow();
-                        MetaRecord.populateRowFromDBObject(obj, newRow);
-                        Row oldRow = metaIdIndex.getRow(session, id);
-                        if (oldRow != null) {
-                            meta.updateRow(session, oldRow, newRow);
-                        }
+            int id = obj.getId();
+            if (id > 0) {
+                if (!starting && !obj.isTemporary()) {
+                    Row newRow = meta.getTemplateRow();
+                    MetaRecord.populateRowFromDBObject(obj, newRow);
+                    Row oldRow = metaIdIndex.getRow(session, id);
+                    if (oldRow != null) {
+                        meta.updateRow(session, oldRow, newRow);
                     }
-                    // for temporary objects
+                }
+                // for temporary objects
+                synchronized (objectIds) {
                     objectIds.set(id);
                 }
             }
@@ -2336,7 +2362,7 @@ public class Database implements DataHandler {
         return lockMode;
     }
 
-    public synchronized void setCloseDelay(int value) {
+    public void setCloseDelay(int value) {
         this.closeDelay = value;
     }
 

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

@@ -7,6 +7,7 @@ package org.h2.engine;
 
 import java.util.ArrayDeque;
 import java.util.ArrayList;
+import java.util.BitSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -162,6 +163,12 @@ public class Session extends SessionWithState implements TransactionStore.Rollba
     private State state = State.INIT;
     private long startStatement = -1;
 
+    /**
+     * Set of database object ids to be released at the end of transaction
+     */
+    private final BitSet idsToRelease = new BitSet();
+
+
     public Session(Database database, User user, int id) {
         this.database = database;
         this.queryTimeout = database.getSettings().maxQueryTimeout;
@@ -627,6 +634,10 @@ public class Session extends SessionWithState implements TransactionStore.Rollba
         return command;
     }
 
+    void releaseDatabaseObjectId(int id) {
+        idsToRelease.set(id);
+    }
+
     public Database getDatabase() {
         return database;
     }
@@ -746,6 +757,8 @@ public class Session extends SessionWithState implements TransactionStore.Rollba
             removeLobMap = null;
         }
         unlockAll();
+        database.releaseDatabaseObjectIds(idsToRelease);
+        idsToRelease.clear();
     }
 
     /**
@@ -853,6 +866,7 @@ public class Session extends SessionWithState implements TransactionStore.Rollba
 
                 removeTemporaryLobs(false);
                 cleanTempTables(true);
+                commit(true);       // temp table rempval may have opened new transaction
                 if (undoLog != null) {
                     undoLog.clear();
                 }
@@ -961,27 +975,35 @@ public class Session extends SessionWithState implements TransactionStore.Rollba
 
     private void cleanTempTables(boolean closeSession) {
         if (localTempTables != null && localTempTables.size() > 0) {
-            synchronized (database) {
-                Iterator<Table> it = localTempTables.values().iterator();
-                while (it.hasNext()) {
-                    Table table = it.next();
-                    if (closeSession || table.getOnCommitDrop()) {
-                        modificationId++;
-                        table.setModified();
-                        it.remove();
-                        // Exception thrown in org.h2.engine.Database.removeMeta
-                        // if line below is missing with TestDeadlock
-                        database.lockMeta(this);
-                        table.removeChildrenAndResources(this);
-                        if (closeSession) {
-                            // need to commit, otherwise recovery might
-                            // ignore the table removal
-                            database.commit(this);
-                        }
-                    } else if (table.getOnCommitTruncate()) {
-                        table.truncate(this);
-                    }
+            if (database.isMVStore()) {
+                _cleanTempTables(closeSession);
+            } else {
+                synchronized (database) {
+                    _cleanTempTables(closeSession);
+                }
+            }
+        }
+    }
+
+    private void _cleanTempTables(boolean closeSession) {
+        Iterator<Table> it = localTempTables.values().iterator();
+        while (it.hasNext()) {
+            Table table = it.next();
+            if (closeSession || table.getOnCommitDrop()) {
+                modificationId++;
+                table.setModified();
+                it.remove();
+                // Exception thrown in org.h2.engine.Database.removeMeta
+                // if line below is missing with TestDeadlock
+                database.lockMeta(this);
+                table.removeChildrenAndResources(this);
+                if (closeSession) {
+                    // need to commit, otherwise recovery might
+                    // ignore the table removal
+                    database.commit(this);
                 }
+            } else if (table.getOnCommitTruncate()) {
+                table.truncate(this);
             }
         }
     }

h2/src/main/org/h2/mvstore/db/MVTable.java

@@ -809,16 +809,18 @@ public class MVTable extends TableBase {
         }
         database.getStore().removeTable(this);
         super.removeChildrenAndResources(session);
-        // go backwards because database.removeIndex will
-        // call table.removeIndex
+        // remove scan index (at position 0 on the list) last
         while (indexes.size() > 1) {
             Index index = indexes.get(1);
+            index.remove(session);
             if (index.getName() != null) {
                 database.removeSchemaObject(session, index);
             }
             // needed for session temporary indexes
             indexes.remove(index);
         }
+        primaryIndex.remove(session);
+        indexes.clear();
         if (SysProperties.CHECK) {
             for (SchemaObject obj : database
                     .getAllSchemaObjects(DbObject.INDEX)) {
@@ -829,8 +831,6 @@ public class MVTable extends TableBase {
                 }
             }
         }
-        primaryIndex.remove(session);
-        database.removeMeta(session, getId());
         close(session);
         invalidate();
     }

h2/src/main/org/h2/mvstore/tx/TransactionStore.java

@@ -16,7 +16,6 @@ import org.h2.mvstore.Cursor;
 import org.h2.mvstore.DataUtils;
 import org.h2.mvstore.MVMap;
 import org.h2.mvstore.MVStore;
-import org.h2.mvstore.Page;
 import org.h2.mvstore.WriteBuffer;
 import org.h2.mvstore.type.DataType;
 import org.h2.mvstore.type.ObjectDataType;
@@ -416,7 +415,7 @@ public class TransactionStore {
      * @param map the map
      */
     <K, V> void removeMap(TransactionMap<K, V> map) {
-        store.removeMap(map.map, true);
+        store.removeMap(map.map, false);
     }
 
     /**

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

@@ -1129,7 +1129,7 @@ public class MetaTable extends Table {
                     add(rows, "info.PAGE_COUNT",
                             Long.toString(pageCount));
                     add(rows, "info.PAGE_SIZE",
-                            Integer.toString(pageSize));
+                            Integer.toString(mvStore.getPageSplitSize()));
                     add(rows, "info.CACHE_MAX_SIZE",
                             Integer.toString(mvStore.getCacheSize()));
                     add(rows, "info.CACHE_SIZE",

h2/src/test/org/h2/test/db/TestMultiThreadedKernel.java

@@ -42,14 +42,6 @@ public class TestMultiThreadedKernel extends TestDb {
         TestBase.createCaller().init().test();
     }
 
-    @Override
-    public boolean isEnabled() {
-        if (config.mvStore) { // FIXME can't see why test should not work in MVStore mode
-            return false;
-        }
-        return true;
-    }
-
     @Override
     public void test() throws Exception {
         deleteDb("multiThreadedKernel");