Merge pull request #1217 from h2database/exception-handling

Postpone session.endStatement() until after commit.

h2/src/main/org/h2/command/Command.java

@@ -151,20 +151,15 @@ public abstract class Command implements CommandInterface {
 
     @Override
     public void stop() {
-        session.endStatement();
         session.setCurrentCommand(null, false);
         if (!isTransactional()) {
             session.commit(true);
         } else if (session.getAutoCommit()) {
             session.commit(false);
-        } else if (session.getDatabase().isMultiThreaded()) {
-            Database db = session.getDatabase();
-            if (db != null) {
-                if (db.getLockMode() == Constants.LOCK_MODE_READ_COMMITTED) {
-                    session.unlockReadLocks();
-                }
-            }
+        } else {
+            session.unlockReadLocks();
         }
+        session.endStatement();
         if (trace.isInfoEnabled() && startTimeNanos > 0) {
             long timeMillis = (System.nanoTime() - startTimeNanos) / 1000 / 1000;
             if (timeMillis > Constants.SLOW_QUERY_LIMIT_MS) {
@@ -260,6 +255,7 @@ public abstract class Command implements CommandInterface {
             Session.Savepoint rollback = session.setSavepoint();
             session.startStatementWithinTransaction();
             session.setCurrentCommand(this, generatedKeysRequest);
+            DbException ex = null;
             try {
                 while (true) {
                     database.checkPowerOff();
@@ -289,18 +285,29 @@ public abstract class Command implements CommandInterface {
                     database.shutdownImmediately();
                     throw e;
                 }
-                database.checkPowerOff();
-                if (s.getErrorCode() == ErrorCode.DEADLOCK_1) {
-                    session.rollback();
-                } else {
-                    session.rollbackTo(rollback, false);
+                try {
+                    database.checkPowerOff();
+                    if (s.getErrorCode() == ErrorCode.DEADLOCK_1) {
+                        session.rollback();
+                    } else {
+                        session.rollbackTo(rollback, false);
+                    }
+                } catch (Throwable nested) {
+                    e.addSuppressed(nested);
                 }
+                ex = e;
                 throw e;
             } finally {
                 try {
                     if (callStop) {
                         stop();
                     }
+                } catch (Throwable nested) {
+                    if (ex == null) {
+                        throw nested;
+                    } else {
+                        ex.addSuppressed(nested);
+                    }
                 } finally {
                     if (writing) {
                         database.afterWriting();

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

@@ -943,19 +943,14 @@ public class Session extends SessionWithState implements TransactionStore.Rollba
      * READ_COMMITTED.
      */
     public void unlockReadLocks() {
-        if (database.isMVStore()) {
-            // MVCC: keep shared locks (insert / update / delete)
-            return;
-        }
-        // locks is modified in the loop
-        for (int i = 0; i < locks.size(); i++) {
-            Table t = locks.get(i);
-            if (!t.isLockedExclusively()) {
-                synchronized (database) {
+        if (!database.isMVStore() && database.isMultiThreaded() &&
+                database.getLockMode() == Constants.LOCK_MODE_READ_COMMITTED) {
+            for (Iterator<Table> iter = locks.iterator(); iter.hasNext(); ) {
+                Table t = iter.next();
+                if (!t.isLockedExclusively()) {
                     t.unlock(this);
-                    locks.remove(i);
+                    iter.remove();
                 }
-                i--;
             }
         }
     }

h2/src/main/org/h2/message/DbException.java

@@ -33,6 +33,10 @@ public class DbException extends RuntimeException {
 
     private static final Properties MESSAGES = new Properties();
 
+    public static final SQLException SQL_OOME =
+            new SQLException("OutOfMemoryError", "HY000", ErrorCode.OUT_OF_MEMORY, new OutOfMemoryError());
+    private static final DbException OOME = new DbException(SQL_OOME);
+
     private Object source;
 
     static {
@@ -288,22 +292,32 @@ public class DbException extends RuntimeException {
      * @return the exception object
      */
     public static DbException convert(Throwable e) {
-        if (e instanceof DbException) {
-            return (DbException) e;
-        } else if (e instanceof SQLException) {
-            return new DbException((SQLException) e);
-        } else if (e instanceof InvocationTargetException) {
-            return convertInvocation((InvocationTargetException) e, null);
-        } else if (e instanceof IOException) {
-            return get(ErrorCode.IO_EXCEPTION_1, e, e.toString());
-        } else if (e instanceof OutOfMemoryError) {
-            return get(ErrorCode.OUT_OF_MEMORY, e);
-        } else if (e instanceof StackOverflowError || e instanceof LinkageError) {
+        try {
+            if (e instanceof DbException) {
+                return (DbException) e;
+            } else if (e instanceof SQLException) {
+                return new DbException((SQLException) e);
+            } else if (e instanceof InvocationTargetException) {
+                return convertInvocation((InvocationTargetException) e, null);
+            } else if (e instanceof IOException) {
+                return get(ErrorCode.IO_EXCEPTION_1, e, e.toString());
+            } else if (e instanceof OutOfMemoryError) {
+                return get(ErrorCode.OUT_OF_MEMORY, e);
+            } else if (e instanceof StackOverflowError || e instanceof LinkageError) {
+                return get(ErrorCode.GENERAL_ERROR_1, e, e.toString());
+            } else if (e instanceof Error) {
+                throw (Error) e;
+            }
             return get(ErrorCode.GENERAL_ERROR_1, e, e.toString());
-        } else if (e instanceof Error) {
-            throw (Error) e;
+        } catch (Throwable ex) {
+            try {
+                DbException dbException = new DbException(new SQLException("GeneralError", "HY000", ErrorCode.GENERAL_ERROR_1, e));
+                dbException.addSuppressed(ex);
+                return dbException;
+            } catch (OutOfMemoryError ignore) {
+                return OOME;
+            }
         }
-        return get(ErrorCode.GENERAL_ERROR_1, e, e.toString());
     }
 
     /**

h2/src/main/org/h2/message/TraceObject.java

@@ -10,6 +10,7 @@ import java.sql.SQLException;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicIntegerArray;
 
+import org.h2.api.ErrorCode;
 import org.h2.util.StringUtils;
 
 /**
@@ -367,7 +368,11 @@ public class TraceObject {
             }
         } catch(Throwable ignore) {
             if (e == null) {
-                e = new SQLException("", "HY000", ex);
+                try {
+                    e = new SQLException("GeneralError", "HY000", ErrorCode.GENERAL_ERROR_1, ex);
+                } catch (OutOfMemoryError ignored) {
+                    return DbException.SQL_OOME;
+                }
             }
             e.addSuppressed(ignore);
         }

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

@@ -58,7 +58,7 @@ public class MVPrimaryIndex extends BaseIndex {
         Transaction t = mvTable.getTransactionBegin();
         dataMap = t.openMap(mapName, keyType, valueType);
         t.commit();
-        if (!table.isPersistData()) {
+        if (!table.isPersistData() || !indexType.isPersistent()) {
             dataMap.map.setVolatile(true);
         }
         Value k = dataMap.map.lastKey();    // include uncommitted keys as well
@@ -113,7 +113,8 @@ public class MVPrimaryIndex extends BaseIndex {
         }
 
         TransactionMap<Value, Value> map = getMap(session);
-        Value key = ValueLong.get(row.getKey());
+        long rowKey = row.getKey();
+        Value key = ValueLong.get(rowKey);
         try {
             Value oldValue = map.putIfAbsent(key, ValueArray.get(row.getValueList()));
             if (oldValue != null) {
@@ -135,8 +136,9 @@ public class MVPrimaryIndex extends BaseIndex {
         }
         // because it's possible to directly update the key using the _rowid_
         // syntax
-        if (row.getKey() > lastKey.get()) {
-            lastKey.set(row.getKey());
+        long last;
+        while (rowKey > (last = lastKey.get())) {
+            if(lastKey.compareAndSet(last, rowKey)) break;
         }
     }
 
@@ -223,33 +225,27 @@ public class MVPrimaryIndex extends BaseIndex {
 
     @Override
     public Cursor find(Session session, SearchRow first, SearchRow last) {
-        ValueLong min, max;
-        if (first == null) {
-            min = ValueLong.MIN;
-        } else if (mainIndexColumn < 0) {
-            min = ValueLong.get(first.getKey());
-        } else {
-            ValueLong v = (ValueLong) first.getValue(mainIndexColumn);
-            if (v == null) {
-                min = ValueLong.get(first.getKey());
-            } else {
-                min = v;
-            }
-        }
-        if (last == null) {
-            max = ValueLong.MAX;
-        } else if (mainIndexColumn < 0) {
-            max = ValueLong.get(last.getKey());
+        ValueLong min = extractPKFromRow(first, ValueLong.MIN);
+        ValueLong max = extractPKFromRow(last, ValueLong.MAX);
+        TransactionMap<Value, Value> map = getMap(session);
+        return new MVStoreCursor(session, map.entryIterator(min, max));
+    }
+
+    private ValueLong extractPKFromRow(SearchRow row, ValueLong defaultValue) {
+        ValueLong result;
+        if (row == null) {
+            result = defaultValue;
+        } else if (mainIndexColumn == SearchRow.ROWID_INDEX) {
+            result = ValueLong.get(row.getKey());
         } else {
-            ValueLong v = (ValueLong) last.getValue(mainIndexColumn);
+            ValueLong v = (ValueLong) row.getValue(mainIndexColumn);
             if (v == null) {
-                max = ValueLong.get(last.getKey());
+                result = ValueLong.get(row.getKey());
             } else {
-                max = v;
+                result = v;
             }
         }
-        TransactionMap<Value, Value> map = getMap(session);
-        return new MVStoreCursor(session, map.entryIterator(min, max));
+        return result;
     }
 
     @Override

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

@@ -5,7 +5,6 @@
  */
 package org.h2.mvstore.db;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
@@ -421,9 +420,8 @@ public final class MVSecondaryIndex extends BaseIndex implements MVIndex {
             }
             key = first ? map.higherKey(key) : map.lowerKey(key);
         }
-        ArrayList<Value> list = new ArrayList<>(1);
-        list.add(key);
-        MVStoreCursor cursor = new MVStoreCursor(session, list.iterator(), null);
+        MVStoreCursor cursor = new MVStoreCursor(session,
+                                Collections.singletonList(key).iterator(), null);
         cursor.next();
         return cursor;
     }
@@ -544,7 +542,6 @@ public final class MVSecondaryIndex extends BaseIndex implements MVIndex {
         public boolean previous() {
             throw DbException.getUnsupportedException("previous");
         }
-
     }
 
 }

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

@@ -427,8 +427,9 @@ public class MVTable extends TableBase {
     @Override
     public void unlock(Session s) {
         if (database != null) {
-            traceLock(s, lockExclusiveSession == s, TraceLockEvent.TRACE_LOCK_UNLOCK, NO_EXTRA_INFO);
-            if (lockExclusiveSession == s) {
+            boolean wasLocked = lockExclusiveSession == s;
+            traceLock(s, wasLocked, TraceLockEvent.TRACE_LOCK_UNLOCK, NO_EXTRA_INFO);
+            if (wasLocked) {
                 lockSharedSessions.remove(s);
                 lockExclusiveSession = null;
                 if (SysProperties.THREAD_DEADLOCK_DETECTOR) {
@@ -436,18 +437,18 @@ public class MVTable extends TableBase {
                         EXCLUSIVE_LOCKS.get().remove(getName());
                     }
                 }
-            }
-            synchronized (getLockSyncObject()) {
-                if (!lockSharedSessions.isEmpty()) {
-                    lockSharedSessions.remove(s);
-                    if (SysProperties.THREAD_DEADLOCK_DETECTOR) {
-                        if (SHARED_LOCKS.get() != null) {
-                            SHARED_LOCKS.get().remove(getName());
-                        }
+            } else {
+                wasLocked = lockSharedSessions.remove(s) != null;
+                if (SysProperties.THREAD_DEADLOCK_DETECTOR) {
+                    if (SHARED_LOCKS.get() != null) {
+                        SHARED_LOCKS.get().remove(getName());
                     }
                 }
-                if (!waitingSessions.isEmpty()) {
-                    getLockSyncObject().notifyAll();
+            }
+            if (wasLocked && !waitingSessions.isEmpty()) {
+                Object lockSyncObject = getLockSyncObject();
+                synchronized (lockSyncObject) {
+                    lockSyncObject.notifyAll();
                 }
             }
         }

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

@@ -63,14 +63,17 @@ public class TestMemoryUsage extends TestDb {
         }
         deleteDb("memoryUsage");
         conn = getConnection("memoryUsage");
-        eatMemory(4000);
-        for (int i = 0; i < 4000; i++) {
-            Connection c2 = getConnection("memoryUsage");
-            c2.createStatement();
-            c2.close();
+        try {
+            eatMemory(4000);
+            for (int i = 0; i < 4000; i++) {
+                Connection c2 = getConnection("memoryUsage");
+                c2.createStatement();
+                c2.close();
+            }
+        } finally {
+            freeMemory();
+            conn.close();
         }
-        freeMemory();
-        conn.close();
     }
 
     private void testCreateDropLoop() throws SQLException {
@@ -141,8 +144,8 @@ public class TestMemoryUsage extends TestDb {
                 }
             }
         } finally {
-            conn.close();
             freeMemory();
+            conn.close();
         }
     }