Merge pull request #1673 from h2database/issue_1672

Deadlock on MVStore closure due to OOM

h2/src/main/org/h2/mvstore/MVStore.java

@@ -185,7 +185,7 @@ public class MVStore implements AutoCloseable {
 
     private volatile boolean reuseSpace = true;
 
-    private final AtomicInteger state = new AtomicInteger();
+    private volatile int state;
 
     private final FileStore fileStore;
 
@@ -960,10 +960,11 @@ public class MVStore implements AutoCloseable {
         // isClosed() would wait until closure is done and then  we jump out of the loop.
         // This is a subtle difference between !isClosed() and isOpen().
         while (!isClosed()) {
-            if (state.compareAndSet(STATE_OPEN, STATE_STOPPING)) {
-                try {
-                    stopBackgroundThread(normalShutdown);
-                    storeLock.lock();
+            stopBackgroundThread(normalShutdown);
+            storeLock.lock();
+            try {
+                if (state == STATE_OPEN) {
+                    state = STATE_STOPPING;
                     try {
                         try {
                             if (normalShutdown && fileStore != null && !fileStore.isReadOnly()) {
@@ -979,7 +980,7 @@ public class MVStore implements AutoCloseable {
                                 shrinkFileIfPossible(0);
                             }
 
-                            state.set(STATE_CLOSING);
+                            state = STATE_CLOSING;
 
                             // release memory early - this is important when called
                             // because of out of memory
@@ -1000,11 +1001,11 @@ public class MVStore implements AutoCloseable {
                             }
                         }
                     } finally {
-                        storeLock.unlock();
+                        state = STATE_CLOSED;
                     }
-                } finally {
-                    state.set(STATE_CLOSED);
                 }
+            } finally {
+                storeLock.unlock();
             }
         }
     }
@@ -2806,7 +2807,7 @@ public class MVStore implements AutoCloseable {
     }
 
     private boolean isOpen() {
-        return state.get() == STATE_OPEN;
+        return state == STATE_OPEN;
     }
 
     /**
@@ -2817,27 +2818,17 @@ public class MVStore implements AutoCloseable {
         if (isOpen()) {
             return false;
         }
-        int millis = 1;
-        while (state.get() != STATE_CLOSED) {
-            /*
-             * We need to wait for completion of close procedure. This is
-             * required because otherwise database may be closed too early while
-             * underlying storage still has unreleased resources. The quickly
-             * following connection attempts fail with The file is locked
-             * exception.
-             */
-            try {
-                Thread.sleep(millis++);
-            } catch (InterruptedException e) {
-                Thread.currentThread().interrupt();
-                break;
-            }
+        storeLock.lock();
+        try {
+            assert state == STATE_CLOSED;
+            return true;
+        } finally {
+            storeLock.unlock();
         }
-        return true;
     }
 
     private boolean isOpenOrStopping() {
-        return state.get() <= STATE_STOPPING;
+        return state <= STATE_STOPPING;
     }
 
     private void stopBackgroundThread(boolean waitForIt) {

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

@@ -25,31 +25,35 @@ public class TestAnalyzeTableTx extends TestDb {
 
     @Override
     public boolean isEnabled() {
-        if (config.networked || config.big) {
-            return false;
-        }
-        return true;
+        return !config.networked && !config.big;
     }
 
     @Override
     public void test() throws Exception {
         deleteDb(getTestName());
-        Connection shared = getConnection(getTestName());
-        Statement statement = shared.createStatement();
-        statement.executeUpdate("DROP TABLE IF EXISTS TEST");
-        statement.executeUpdate("CREATE TABLE TEST(ID INT PRIMARY KEY)");
         Connection[] connections = new Connection[C];
-        for (int i = 0; i < C; i++) {
-            Connection c = getConnection(getTestName());
-            c.createStatement().executeUpdate("INSERT INTO TEST VALUES (" + i + ')');
-            connections[i] = c;
-        }
-        try (ResultSet rs = statement.executeQuery("SELECT * FROM TEST")) {
+        try (Connection shared = getConnection(getTestName())) {
+            Statement statement = shared.createStatement();
+            statement.executeUpdate("DROP TABLE IF EXISTS TEST");
+            statement.executeUpdate("CREATE TABLE TEST(ID INT PRIMARY KEY)");
             for (int i = 0; i < C; i++) {
-                if (!rs.next())
-                    throw new Exception("next");
-                if (rs.getInt(1) != i)
-                    throw new Exception(Integer.toString(i));
+                Connection c = getConnection(getTestName());
+                c.createStatement().executeUpdate("INSERT INTO TEST VALUES (" + i + ')');
+                connections[i] = c;
+            }
+            try (ResultSet rs = statement.executeQuery("SELECT * FROM TEST")) {
+                for (int i = 0; i < C; i++) {
+                    if (!rs.next())
+                        throw new Exception("next");
+                    if (rs.getInt(1) != i)
+                        throw new Exception(Integer.toString(i));
+                }
+            }
+        } finally {
+            for (Connection connection : connections) {
+                if (connection != null) {
+                    try { connection.close(); } catch (Throwable ignore) {/**/}
+                }
             }
         }
     }

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

@@ -506,8 +506,7 @@ public class TestMultiThread extends TestDb implements Runnable {
 
         @Override
         public void run() {
-            try {
-                Connection c = getConnection("concurrentUpdate2;LOCK_TIMEOUT=10000");
+            try (Connection c = getConnection("concurrentUpdate2;LOCK_TIMEOUT=10000")) {
                 PreparedStatement ps = c.prepareStatement("UPDATE TEST SET V = ? WHERE " + column + " = ?");
                 for (int test = 0; test < 1000; test++) {
                     for (int i = 0; i < 16; i++) {
@@ -524,34 +523,36 @@ public class TestMultiThread extends TestDb implements Runnable {
 
     private void testConcurrentUpdate2() throws Exception {
         deleteDb("concurrentUpdate2");
-        Connection c = getConnection("concurrentUpdate2");
-        Statement s = c.createStatement();
-        s.execute("CREATE TABLE TEST(A INT, B INT, V INT, PRIMARY KEY(A, B))");
-        PreparedStatement ps = c.prepareStatement("INSERT INTO TEST VALUES (?, ?, ?)");
-        for (int i = 0; i < 16; i++) {
-            for (int j = 0; j < 16; j++) {
-                ps.setInt(1, i);
-                ps.setInt(2, j);
-                ps.setInt(3, 0);
-                ps.executeUpdate();
+        try (Connection c = getConnection("concurrentUpdate2")) {
+            Statement s = c.createStatement();
+            s.execute("CREATE TABLE TEST(A INT, B INT, V INT, PRIMARY KEY(A, B))");
+            PreparedStatement ps = c.prepareStatement("INSERT INTO TEST VALUES (?, ?, ?)");
+            for (int i = 0; i < 16; i++) {
+                for (int j = 0; j < 16; j++) {
+                    ps.setInt(1, i);
+                    ps.setInt(2, j);
+                    ps.setInt(3, 0);
+                    ps.executeUpdate();
+                }
             }
-        }
-        ConcurrentUpdate2 a = new ConcurrentUpdate2("A");
-        ConcurrentUpdate2 b = new ConcurrentUpdate2("B");
-        a.start();
-        b.start();
-        a.join();
-        b.join();
-        deleteDb("concurrentUpdate2");
-        Throwable e = a.exception;
-        if (e == null) {
-            e = b.exception;
-        }
-        if (e != null) {
-            if (e instanceof Exception) {
-                throw (Exception) e;
+            ConcurrentUpdate2 a = new ConcurrentUpdate2("A");
+            ConcurrentUpdate2 b = new ConcurrentUpdate2("B");
+            a.start();
+            b.start();
+            a.join();
+            b.join();
+            Throwable e = a.exception;
+            if (e == null) {
+                e = b.exception;
             }
-            throw (Error) e;
+            if (e != null) {
+                if (e instanceof Exception) {
+                    throw (Exception) e;
+                }
+                throw (Error) e;
+            }
+        } finally {
+            deleteDb("concurrentUpdate2");
         }
     }
 }