From b867bcebdbc7fef3ca94dfa715a547bb21a50151 Mon Sep 17 00:00:00 2001
From: Aurora Lahtela <24460436+AuroraLS3@users.noreply.github.com>
Date: Sat, 17 Feb 2024 09:19:38 +0200
Subject: [PATCH] Don't save sessions on server shutdown if database already
 closed

Affects issues:
- #3436
---
 .../plan/gathering/ServerShutdownSave.java    | 22 ++++++-------------
 .../plan/gathering/ShutdownSaveTest.java      | 10 ++++++---
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/Plan/common/src/main/java/com/djrapitops/plan/gathering/ServerShutdownSave.java b/Plan/common/src/main/java/com/djrapitops/plan/gathering/ServerShutdownSave.java
index d063075bc..c2c6444f6 100644
--- a/Plan/common/src/main/java/com/djrapitops/plan/gathering/ServerShutdownSave.java
+++ b/Plan/common/src/main/java/com/djrapitops/plan/gathering/ServerShutdownSave.java
@@ -48,7 +48,6 @@ public abstract class ServerShutdownSave {
     private final ErrorLogger errorLogger;
 
     private boolean shuttingDown = false;
-    private boolean startedDatabase = false;
 
     protected ServerShutdownSave(
             Locale locale,
@@ -90,7 +89,7 @@ public abstract class ServerShutdownSave {
 
     private Optional<Future<?>> attemptSave(Collection<ActiveSession> activeSessions) {
         try {
-            return Optional.of(saveActiveSessions(finishSessions(activeSessions, System.currentTimeMillis())));
+            return saveActiveSessions(finishSessions(activeSessions, System.currentTimeMillis()));
         } catch (DBInitException e) {
             errorLogger.error(e, ErrorContext.builder()
                     .whatToDo("Find the sessions in the error file and save them manually or ignore. Report & delete the error file after.")
@@ -101,20 +100,19 @@ public abstract class ServerShutdownSave {
         } catch (IllegalStateException ignored) {
             /* Database is not initialized */
             return Optional.empty();
-        } finally {
-            closeDatabase(dbSystem.getDatabase());
         }
     }
 
-    private Future<?> saveActiveSessions(Collection<FinishedSession> finishedSessions) {
+    private Optional<Future<?>> saveActiveSessions(Collection<FinishedSession> finishedSessions) {
         Database database = dbSystem.getDatabase();
         if (database.getState() == Database.State.CLOSED) {
-            // Ensure that database is not closed when performing the transaction.
-            startedDatabase = true;
-            database.init();
+            // Don't attempt to save if database is closed, session storage will be handled by
+            // ShutdownDataPreservation instead.
+            // Previously database reboot was attempted, but this could lead to server hang.
+            return Optional.empty();
         }
 
-        return saveSessions(finishedSessions, database);
+        return Optional.of(saveSessions(finishedSessions, database));
     }
 
     Collection<FinishedSession> finishSessions(Collection<ActiveSession> activeSessions, long now) {
@@ -127,10 +125,4 @@ public abstract class ServerShutdownSave {
     private Future<?> saveSessions(Collection<FinishedSession> finishedSessions, Database database) {
         return database.executeTransaction(new ServerShutdownTransaction(finishedSessions));
     }
-
-    private void closeDatabase(Database database) {
-        if (startedDatabase) {
-            database.close();
-        }
-    }
 }
\ No newline at end of file
diff --git a/Plan/common/src/test/java/com/djrapitops/plan/gathering/ShutdownSaveTest.java b/Plan/common/src/test/java/com/djrapitops/plan/gathering/ShutdownSaveTest.java
index 6f594f88a..d9d9209a5 100644
--- a/Plan/common/src/test/java/com/djrapitops/plan/gathering/ShutdownSaveTest.java
+++ b/Plan/common/src/test/java/com/djrapitops/plan/gathering/ShutdownSaveTest.java
@@ -122,9 +122,15 @@ class ShutdownSaveTest {
         Optional<Future<?>> future = underTest.performSave();
         assertTrue(future.isEmpty());
 
-        database.init();
         assertTrue(database.query(SessionQueries.fetchAllSessions()).isEmpty());
+    }
+
+    @Test
+    void sessionsAreNotSavedIfDatabaseIsClosed() {
+        shutdownStatus = true;
         database.close();
+        Optional<Future<?>> future = underTest.performSave();
+        assertTrue(future.isEmpty());
     }
 
     @Test
@@ -134,9 +140,7 @@ class ShutdownSaveTest {
         assertTrue(save.isPresent());
         save.get().get(); // Wait for save to be done, test fails without.
 
-        database.init();
         assertFalse(database.query(SessionQueries.fetchAllSessions()).isEmpty());
-        database.close();
     }
 
     private void placeSessionToCache() {