Issue 335: Could not run DROP ALL OBJECTS DELETE FILES on older databases with…

Issue 335: Could not run DROP ALL OBJECTS DELETE FILES on older databases with CLOB or BLOB data. The problem was that the metadata table was not locked in some cases, so that a rollback could result in a corrupt database in a database if the lob storage was upgraded.

h2/src/docsrc/html/changelog.html

@@ -18,7 +18,12 @@ Change Log
 <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
-<ul><li>The JDBC methods PreparedStatement.setTimestamp, setTime, and setDate with a calendar,
+<ul><li>Issue 335: Could not run DROP ALL OBJECTS DELETE FILES on older databases with CLOB or BLOB data.
+    The problem was that the metadata table was not locked in some cases, so that
+    a rollback could result in a corrupt database in a database if the lob storage was upgraded.
+</li><li>Source code switching using //## has been simplified.
+    The Java 1.5 tag has been removed because is no longer needed.
+</li><li>The JDBC methods PreparedStatement.setTimestamp, setTime, and setDate with a calendar,
     and the methods ResultSet.getTimestamp, getTime, and getDate with a calendar converted
     the value in the wrong way, so that for some timestamps the converted value was wrong
     (where summertime starts, one hour per year).

h2/src/main/org/h2/command/ddl/AlterTableAddConstraint.java

@@ -89,6 +89,7 @@ public class AlterTableAddConstraint extends SchemaCommand {
             throw DbException.get(ErrorCode.CONSTRAINT_ALREADY_EXISTS_1, constraintName);
         }
         session.getUser().checkRight(table, Right.ALL);
+        db.lockMeta(session);
         table.lock(session, true, true);
         Constraint constraint;
         switch (type) {

h2/src/main/org/h2/command/ddl/CreateTable.java

@@ -127,6 +127,10 @@ public class CreateTable extends SchemaCommand {
         data.id = getObjectId();
         data.create = create;
         data.session = session;
+        boolean isSessionTemporary = data.temporary && !data.globalTemporary;
+        if (!isSessionTemporary) {
+            db.lockMeta(session);
+        }
         Table table = getSchema().createTable(data);
         ArrayList<Sequence> sequences = New.arrayList();
         for (Column c : data.columns) {
@@ -140,7 +144,7 @@ public class CreateTable extends SchemaCommand {
             }
         }
         table.setComment(comment);
-        if (data.temporary && !data.globalTemporary) {
+        if (isSessionTemporary) {
             if (onCommitDrop) {
                 table.setOnCommitDrop(true);
             }
@@ -149,6 +153,7 @@ public class CreateTable extends SchemaCommand {
             }
             session.addLocalTempTable(table);
         } else {
+            db.lockMeta(session);
             db.addSchemaObject(session, table);
         }
         try {

h2/src/main/org/h2/command/ddl/DropDatabase.java

@@ -45,6 +45,7 @@ public class DropDatabase extends DefineCommand {
         session.getUser().checkAdmin();
         session.commit(true);
         Database db = session.getDatabase();
+        db.lockMeta(session);
         // TODO local temp tables are not removed
         for (Schema schema : db.getAllSchemas()) {
             if (schema.canDrop()) {

h2/src/main/org/h2/command/ddl/DropTable.java

@@ -99,6 +99,7 @@ public class DropTable extends SchemaCommand {
         if (table != null) {
             table.setModified();
             Database db = session.getDatabase();
+            db.lockMeta(session);
             db.removeSchemaObject(session, table);
         }
         if (next != null) {

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

@@ -614,6 +614,7 @@ public class Database implements DataHandler {
             if (settings.get(name) == null) {
                 Setting setting = new Setting(this, allocateObjectId(), name);
                 setting.setIntValue(Constants.BUILD_ID);
+                lockMeta(systemSession);
                 addDatabaseObject(systemSession, setting);
             }
             // mark all ids used in the page store
@@ -627,6 +628,7 @@ public class Database implements DataHandler {
                 }
             }
         }
+        getLobStorage().init();
         systemSession.commit(true);
         trace.info("opened {0}", databaseName);
         afterWriting();
@@ -710,7 +712,9 @@ public class Database implements DataHandler {
             MetaRecord rec = new MetaRecord(obj);
             rec.setRecord(r);
             objectIds.set(id);
-            meta.lock(session, true, true);
+            if (SysProperties.CHECK) {
+                verifyMetaLocked(session);
+            }
             meta.addRow(session, r);
             if (isMultiVersion()) {
                 // TODO this should work without MVCC, but avoid risks at the moment
@@ -719,6 +723,27 @@ public class Database implements DataHandler {
         }
     }
 
+    public void verifyMetaLocked(Session session) {
+        if (!lockMeta(session) && lockMode != 0) {
+            throw DbException.throwInternalError();
+        }
+    }
+
+    /**
+     * Lock the metadata table for updates.
+     *
+     * @param session the session
+     * @return whether it was already locked before by this session
+     */
+    public synchronized boolean lockMeta(Session session) {
+        if (meta == null) {
+            return true;
+        }
+        boolean wasLocked = meta.isLockedExclusivelyBy(session);
+        meta.lock(session, true, true);
+        return wasLocked;
+    }
+
     /**
      * Remove the given object from the meta data.
      *
@@ -729,10 +754,14 @@ public class Database implements DataHandler {
         if (id > 0 && !starting) {
             SearchRow r = meta.getTemplateSimpleRow(false);
             r.setValue(0, ValueInt.get(id));
-            boolean wasLocked = meta.isLockedExclusivelyBy(session);
-            meta.lock(session, true, true);
+            boolean wasLocked = lockMeta(session);
             Cursor cursor = metaIdIndex.find(session, r, r);
             if (cursor.next()) {
+                if (SysProperties.CHECK) {
+                    if (lockMode != 0 && !wasLocked) {
+                        throw DbException.throwInternalError();
+                    }
+                }
                 Row found = cursor.get();
                 meta.removeRow(session, found);
                 if (isMultiVersion()) {
@@ -798,6 +827,7 @@ public class Database implements DataHandler {
         if (id > 0 && !starting) {
             checkWritingAllowed();
         }
+        lockMeta(session);
         obj.getSchema().add(obj);
         addMeta(session, obj);
     }
@@ -824,6 +854,7 @@ public class Database implements DataHandler {
         if (SysProperties.CHECK && map.get(name) != null) {
             DbException.throwInternalError("object already exists");
         }
+        lockMeta(session);
         addMeta(session, obj);
         map.put(name, obj);
     }
@@ -1131,6 +1162,7 @@ public class Database implements DataHandler {
                 try {
                     pageStore.checkpoint();
                     if (!readOnly) {
+                        lockMeta(pageStore.getSystemSession());
                         pageStore.compact(compactMode);
                     }
                 } catch (DbException e) {
@@ -1357,6 +1389,7 @@ public class Database implements DataHandler {
      * @param obj the database object
      */
     public synchronized void update(Session session, DbObject obj) {
+        lockMeta(session);
         int id = obj.getId();
         removeMeta(session, id);
         addMeta(session, obj);
@@ -1413,6 +1446,7 @@ public class Database implements DataHandler {
         }
         obj.checkRename();
         int id = obj.getId();
+        lockMeta(session);
         removeMeta(session, id);
         map.remove(obj.getName());
         obj.rename(newName);
@@ -1479,6 +1513,7 @@ public class Database implements DataHandler {
             DbException.throwInternalError("not found: " + objName);
         }
         Comment comment = findComment(obj);
+        lockMeta(session);
         if (comment != null) {
             removeDatabaseObject(session, comment);
         }
@@ -1550,6 +1585,7 @@ public class Database implements DataHandler {
             }
         }
         checkWritingAllowed();
+        lockMeta(session);
         Comment comment = findComment(obj);
         if (comment != null) {
             removeDatabaseObject(session, comment);
@@ -1592,6 +1628,7 @@ public class Database implements DataHandler {
     }
 
     public synchronized void setMasterUser(User user) {
+        lockMeta(systemSession);
         addDatabaseObject(systemSession, user);
         systemSession.commit(true);
     }

h2/src/main/org/h2/schema/Schema.java

@@ -529,6 +529,9 @@ public class Schema extends DbObjectBase {
      */
     public Table createTable(CreateTableData data) {
         synchronized (database) {
+            if (!data.temporary || data.globalTemporary) {
+                database.lockMeta(data.session);
+            }
             data.schema = this;
             if (data.tableEngine != null) {
                 TableEngine engine;

h2/src/main/org/h2/store/PageStore.java

@@ -1674,6 +1674,11 @@ public class PageStore implements CacheWriter {
         if (table.isTemporary()) {
             options += "temp";
         }
+        if (SysProperties.CHECK) {
+            if (!table.isTemporary()) {
+                database.verifyMetaLocked(session);
+            }
+        }
         options += ",";
         if (index instanceof PageDelegateIndex) {
             options += "d";
@@ -1696,6 +1701,11 @@ public class PageStore implements CacheWriter {
      * @param session the session
      */
     public synchronized void removeMeta(Index index, Session session) {
+        if (SysProperties.CHECK) {
+            if (!index.getTable().isTemporary()) {
+                database.verifyMetaLocked(session);
+            }
+        }
         if (!recoveryRunning) {
             removeMetaIndex(index, session);
             metaObjects.remove(index.getId());
@@ -1896,4 +1906,8 @@ public class PageStore implements CacheWriter {
         return f;
     }
 
+    public Session getSystemSession() {
+        return systemSession;
+    }
+
 }

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

@@ -19,7 +19,6 @@ import org.h2.constant.SysProperties;
 import org.h2.constraint.Constraint;
 import org.h2.constraint.ConstraintReferential;
 import org.h2.engine.Constants;
-import org.h2.engine.Database;
 import org.h2.engine.DbObject;
 import org.h2.engine.Session;
 import org.h2.index.Cursor;
@@ -80,14 +79,6 @@ public class RegularTable extends TableBase {
                 containsLargeObject = true;
             }
         }
-        if (containsLargeObject) {
-            Database db = data.session.getDatabase();
-            if (!db.isStarting()) {
-                // the lob tables are already created
-                // if the database is starting
-                db.getLobStorage().init();
-            }
-        }
         if (data.persistData && database.isPersistent()) {
             mainIndex = new PageDataIndex(this, data.id,
                     IndexColumn.wrap(getColumns()),
@@ -211,6 +202,10 @@ public class RegularTable extends TableBase {
                 column.setPrimaryKey(true);
             }
         }
+        boolean isSessionTemporary = isTemporary() && !isGlobalTemporary();
+        if (!isSessionTemporary) {
+            database.lockMeta(session);
+        }
         Index index;
         if (isPersistIndexes() && indexType.isPersistent()) {
             int mainIndexColumn;
@@ -280,11 +275,10 @@ public class RegularTable extends TableBase {
                 throw e;
             }
         }
-        boolean temporary = isTemporary();
-        index.setTemporary(temporary);
+        index.setTemporary(isTemporary());
         if (index.getCreateSQL() != null) {
             index.setComment(indexComment);
-            if (temporary && !isGlobalTemporary()) {
+            if (isSessionTemporary) {
                 session.addLocalTempTableIndex(index);
             } else {
                 database.addSchemaObject(session, index);
@@ -657,6 +651,7 @@ public class RegularTable extends TableBase {
             // unfortunately, the data is gone on rollback
             truncate(session);
             database.getLobStorage().removeAllForTable(getId());
+            database.lockMeta(session);
         }
         super.removeChildrenAndResources(session);
         // go backwards because database.removeIndex will call table.removeIndex