Synchronize on undoLog when following MVMap get with TransactionMap getValue

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

@@ -933,9 +933,11 @@ public class TransactionStore {
                 long size = 0;
                 Cursor<K, VersionedValue> cursor = map.cursor(null);
                 while (cursor.hasNext()) {
-                    K key = cursor.next();
-                    VersionedValue data = cursor.getValue();
-                    data = getValue(key, readLogId, data);
+                    VersionedValue data;
+                    synchronized (transaction.store.undoLog) {
+                        K key = cursor.next();
+                        data = getValue(key, readLogId, cursor.getValue());
+                    }
                     if (data != null && data.value != null) {
                         size++;
                     }
@@ -1197,8 +1199,10 @@ public class TransactionStore {
         }
 
         private VersionedValue getValue(K key, long maxLog) {
-            VersionedValue data = map.get(key);
-            return getValue(key, maxLog, data);
+            synchronized (transaction.store.undoLog) {
+                VersionedValue data = map.get(key);
+                return getValue(key, maxLog, data);
+            }
         }
 
         /**
@@ -1210,6 +1214,13 @@ public class TransactionStore {
          * @return the value
          */
         VersionedValue getValue(K key, long maxLog, VersionedValue data) {
+            if (MVStore.ASSERT) {
+                if (!Thread.holdsLock(transaction.store.undoLog)) {
+                    throw DataUtils.newIllegalStateException(
+                            DataUtils.ERROR_INTERNAL,
+                            "getValue not invoked while synchronized on undoLog");
+                }
+            }
             while (true) {
                 if (data == null) {
                     // doesn't exist or deleted by a committed transaction
@@ -1229,9 +1240,7 @@ public class TransactionStore {
                 }
                 // get the value before the uncommitted transaction
                 Object[] d;
-                synchronized (transaction.store.undoLog) {
-                    d = transaction.store.undoLog.get(id);
-                }
+                d = transaction.store.undoLog.get(id);
                 if (d == null) {
                     // this entry should be committed or rolled back
                     // in the meantime (the transaction might still be open)
@@ -1441,36 +1450,38 @@ public class TransactionStore {
                 private void fetchNext() {
                     while (cursor.hasNext()) {
                         K k;
-                        try {
-                            k = cursor.next();
-                        } catch (IllegalStateException e) {
-                            // TODO this is a bit ugly
-                            if (DataUtils.getErrorCode(e.getMessage()) ==
-                                    DataUtils.ERROR_CHUNK_NOT_FOUND) {
-                                cursor = map.cursor(currentKey);
-                                // we (should) get the current key again,
-                                // we need to ignore that one
-                                if (!cursor.hasNext()) {
-                                    break;
-                                }
-                                cursor.next();
-                                if (!cursor.hasNext()) {
-                                    break;
-                                }
+                        synchronized (transaction.store.undoLog) {
+                            try {
                                 k = cursor.next();
-                            } else {
-                                throw e;
+                            } catch (IllegalStateException e) {
+                                // TODO this is a bit ugly
+                                if (DataUtils.getErrorCode(e.getMessage()) ==
+                                        DataUtils.ERROR_CHUNK_NOT_FOUND) {
+                                    cursor = map.cursor(currentKey);
+                                    // we (should) get the current key again,
+                                    // we need to ignore that one
+                                    if (!cursor.hasNext()) {
+                                        break;
+                                    }
+                                    cursor.next();
+                                    if (!cursor.hasNext()) {
+                                        break;
+                                    }
+                                    k = cursor.next();
+                                } else {
+                                    throw e;
+                                }
+                            }
+                            final K key = k;
+                            VersionedValue data = cursor.getValue();
+                            data = getValue(key, readLogId, data);
+                            if (data != null && data.value != null) {
+                                @SuppressWarnings("unchecked")
+                                final V value = (V) data.value;
+                                current = new DataUtils.MapEntry<K, V>(key, value);
+                                currentKey = key;
+                                return;
                             }
-                        }
-                        final K key = k;
-                        VersionedValue data = cursor.getValue();
-                        data = getValue(key, readLogId, data);
-                        if (data != null && data.value != null) {
-                            @SuppressWarnings("unchecked")
-                            final V value = (V) data.value;
-                            current = new DataUtils.MapEntry<K, V>(key, value);
-                            currentKey = key;
-                            return;
                         }
                     }
                     current = null;