convert MVStore.state into AtomicInteger, eliminate race on close()

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

@@ -148,23 +148,24 @@ public class MVStore implements AutoCloseable {
     private static final int MARKED_FREE = 10_000_000;
 
     /**
-     * Storage is open.
+     * Store is open.
      */
     private static final int STATE_OPEN = 0;
 
     /**
-     * Storage is stopping now. Background writer thread finishes its work
-     * during this process.
+     * Store is about to close now, but is still operational.
+     * Outstanding store operation by background writer or other thread may be in progress.
+     * New updates must not be initiated, unless they are part of a closing procedure itself.
      */
     private static final int STATE_STOPPING = 1;
 
     /**
-     * Storage is closing now.
+     * Store is closing now, and any operation on it may fail.
      */
     private static final int STATE_CLOSING = 2;
 
     /**
-     * Storage is closed.
+     * Store is closed.
      */
     private static final int STATE_CLOSED = 3;
 
@@ -183,7 +184,7 @@ public class MVStore implements AutoCloseable {
 
     private volatile boolean reuseSpace = true;
 
-    private volatile int state;
+    private final AtomicInteger state = new AtomicInteger();
 
     private final FileStore fileStore;
 
@@ -435,7 +436,7 @@ public class MVStore implements AutoCloseable {
     }
 
     private void panic(IllegalStateException e) {
-        if (state == STATE_OPEN) {
+        if (isOpen()) {
             handleException(e);
             panicException = e;
             closeImmediately();
@@ -939,27 +940,13 @@ public class MVStore implements AutoCloseable {
      */
     @Override
     public void close() {
-        if (isClosed()) {
-            return;
-        }
-        FileStore f = fileStore;
-        if (f != null && !f.isReadOnly()) {
-            stopBackgroundThread();
-            for (MVMap<?, ?> map : maps.values()) {
-                if (map.isClosed()) {
-                    if (meta.remove(MVMap.getMapRootKey(map.getId())) != null) {
-                        markMetaChanged();
-                    }
-                }
-            }
-            commit();
-        }
         closeStore(true);
     }
 
     /**
-     * Close the file and the store, without writing anything. This will stop
-     * the background thread. This method ignores all errors.
+     * Close the file and the store, without writing anything.
+     * This will try to stop the background thread (without waiting for it).
+     * This method ignores all errors.
      */
     public void closeImmediately() {
         try {
@@ -969,20 +956,29 @@ public class MVStore implements AutoCloseable {
         }
     }
 
-    private void closeStore(boolean shrinkIfPossible) {
-        if (isClosed()) {
-            return;
-        }
-        state = STATE_STOPPING;
+    private void closeStore(boolean normalShutdown) {
+        while (!isClosed()) {
+            if (state.compareAndSet(STATE_OPEN, STATE_STOPPING)) {
                 try {
                     stopBackgroundThread();
-            state = STATE_CLOSING;
                     storeLock.lock();
                     try {
                         try {
-                    if (fileStore != null && shrinkIfPossible) {
+                            if (normalShutdown && fileStore != null && !fileStore.isReadOnly()) {
+                                for (MVMap<?, ?> map : maps.values()) {
+                                    if (map.isClosed()) {
+                                        if (meta.remove(MVMap.getMapRootKey(map.getId())) != null) {
+                                            markMetaChanged();
+                                        }
+                                    }
+                                }
+                                commit();
+
                                 shrinkFileIfPossible(0);
                             }
+
+                            state.set(STATE_CLOSING);
+
                             // release memory early - this is important when called
                             // because of out of memory
                             if (cache != null) {
@@ -1005,7 +1001,9 @@ public class MVStore implements AutoCloseable {
                         storeLock.unlock();
                     }
                 } finally {
-            state = STATE_CLOSED;
+                    state.set(STATE_CLOSED);
+                }
+            }
         }
     }
 
@@ -2805,12 +2803,20 @@ public class MVStore implements AutoCloseable {
         }
     }
 
+    private boolean isOpen() {
+        return state.get() == STATE_OPEN;
+    }
+
+    /**
+     * Determine that store is open, or wait for it to be closed (by other thread)
+     * @return true if store is open, false otherwise
+     */
     public boolean isClosed() {
-        if (state == STATE_OPEN) {
+        if (isOpen()) {
             return false;
         }
         int millis = 1;
-        while (state != STATE_CLOSED) {
+        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
@@ -2829,7 +2835,7 @@ public class MVStore implements AutoCloseable {
     }
 
     private boolean isOpenOrStopping() {
-        return state <= STATE_STOPPING;
+        return state.get() <= STATE_STOPPING;
     }
 
     private void stopBackgroundThread() {
@@ -2874,7 +2880,7 @@ public class MVStore implements AutoCloseable {
         }
         stopBackgroundThread();
         // start the background thread if needed
-        if (millis > 0 && state == STATE_OPEN) {
+        if (millis > 0 && isOpen()) {
             int sleep = Math.max(1, millis / 10);
             BackgroundWriterThread t =
                     new BackgroundWriterThread(this, sleep,