When the shutdown hook closed the database, the last log file was deleted too early.

h2/src/docsrc/html/changelog.html

@@ -18,7 +18,9 @@ Change Log
 <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
-<ul><li>JdbcConnectionPool: it was possible to set a negative connection pool size.
+<ul><li>When the shutdown hook closed the database, the last log file
+    was deleted too early. This could cause uncommitted changes to be persisted.
+</li><li>JdbcConnectionPool: it was possible to set a negative connection pool size.
 </li><li>Fulltext search did not support table names with a backslash.
 </li><li>The internal IntArray class did not work correctly when initialized with a zero length array.
 </li><li>The H2 Console web application (war file) did only support ASCII characters.

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

@@ -1054,6 +1054,9 @@ public class Database implements DataHandler {
             for (int i = 0; i < all.length; i++) {
                 Session s = all[i];
                 try {
+                    // must roll back, otherwise the session is removed and
+                    // the log file that contains its uncommitted operations as well
+                    s.rollback();
                     s.close();
                 } catch (SQLException e) {
                     traceSystem.getTrace(Trace.SESSION).error("disconnecting #" + s.getId(), e);

h2/src/main/org/h2/log/LogSystem.java

@@ -144,6 +144,9 @@ public class LogSystem {
                 l.setFirstUncommittedPos(LOG_WRITTEN);
             } else if (l.getId() == firstUncommittedLog) {
                 if (firstUncommittedPos == l.getPos()) {
+                    // that means firstUncommittedPos is still
+                    // were it was at the beginning
+                    // and all sessions are committed
                     l.setFirstUncommittedPos(LOG_WRITTEN);
                 } else {
                     l.setFirstUncommittedPos(firstUncommittedPos);

h2/src/test/org/h2/test/synth/TestHalt.java

@@ -101,27 +101,27 @@ public abstract class TestHalt extends TestBase {
     /**
      * Initialize the test.
      */
-    abstract void testInit() throws SQLException;
+    abstract void controllerInit() throws SQLException;
 
     /**
      * Check if the database is consistent after a simulated database crash.
      */
-    abstract void testCheckAfterCrash() throws SQLException;
+    abstract void controllerCheckAfterCrash() throws SQLException;
 
     /**
      * Wait for some time after the application has been started.
      */
-    abstract void testWaitAfterAppStart() throws Exception;
+    abstract void controllerWaitAfterAppStart() throws Exception;
 
     /**
      * Start the application.
      */
-    abstract void appStart() throws SQLException;
+    abstract void processAppStart() throws SQLException;
 
     /**
      * Run the application.
      */
-    abstract void appRun() throws SQLException;
+    abstract void processAppRun() throws SQLException;
 
     public void test() throws SQLException {
         for (int i = 0;; i++) {
@@ -129,7 +129,7 @@ public abstract class TestHalt extends TestBase {
             flags = i >> 4;
             // flags |= FLAG_NO_DELAY; // | FLAG_LOBS;
             try {
-                runTest();
+                controllerTest();
             } catch (Throwable t) {
                 System.out.println("Error: " + t);
                 t.printStackTrace();
@@ -139,37 +139,23 @@ public abstract class TestHalt extends TestBase {
 
     Connection getConnection() throws SQLException {
         org.h2.Driver.load();
-        return DriverManager.getConnection("jdbc:h2:" + baseDir + "/halt", "sa", "sa");
+        String url = "jdbc:h2:" + baseDir + "/halt";
+        // String url = "jdbc:h2:" + baseDir + "/halt;TRACE_LEVEL_FILE=3";
+        return DriverManager.getConnection(url, "sa", "sa");
     }
 
-    /**
-     * Start the program.
-     *
-     * @param args the command line arguments
-     */
-    protected void start(String[] args) throws Exception {
-        if (args.length == 0) {
-            runTest();
-        } else {
-            operations = Integer.parseInt(args[0]);
-            flags = Integer.parseInt(args[1]);
-            value = Integer.parseInt(args[2]);
-            runRandom();
-        }
-    }
-
-    private void runRandom() throws SQLException {
+    void processRunRandom() throws SQLException {
         connect();
         try {
             traceOperation("connected, operations:" + operations + " flags:" + flags + " value:" + value);
-            appStart();
+            processAppStart();
             System.out.println("READY");
             System.out.println("READY");
             System.out.println("READY");
-            appRun();
+            processAppRun();
             traceOperation("done");
         } catch (Exception e) {
-            trace("run", e);
+            traceOperation("run", e);
         }
         disconnect();
     }
@@ -179,7 +165,7 @@ public abstract class TestHalt extends TestBase {
             traceOperation("connecting");
             conn = getConnection();
         } catch (SQLException e) {
-            trace("connect", e);
+            traceOperation("connect", e);
             e.printStackTrace();
             throw e;
         }
@@ -191,7 +177,7 @@ public abstract class TestHalt extends TestBase {
      * @param s the message
      */
     protected void traceOperation(String s) {
-        trace(s, null);
+        traceOperation(s, null);
     }
 
     /**
@@ -200,7 +186,7 @@ public abstract class TestHalt extends TestBase {
      * @param s the message
      * @param e the exception or null
      */
-    protected void trace(String s, Exception e) {
+    protected void traceOperation(String s, Exception e) {
         FileWriter writer = null;
         try {
             File f = new File(baseDir + "/" + TRACE_FILE_NAME);
@@ -219,13 +205,13 @@ public abstract class TestHalt extends TestBase {
         }
     }
 
-    private void runTest() throws Exception {
+    void controllerTest() throws Exception {
         traceOperation("delete database -----------------------------");
         DeleteDbFiles.execute(baseDir, DATABASE_NAME, true);
         new File(baseDir + "/" + TRACE_FILE_NAME).delete();
 
         connect();
-        testInit();
+        controllerInit();
         disconnect();
         for (int i = 0; i < 10; i++) {
             traceOperation("backing up " + sequenceId);
@@ -254,7 +240,7 @@ public abstract class TestHalt extends TestBase {
             } else if (s.startsWith("READY")) {
                 traceOperation("got reply: " + s);
             }
-            testWaitAfterAppStart();
+            controllerWaitAfterAppStart();
             p.destroy();
             try {
                 traceOperation("backing up " + sequenceId);
@@ -262,7 +248,7 @@ public abstract class TestHalt extends TestBase {
                 // new File(BASE_DIR + "/haltSeq" + (sequenceId-20) +
                 // ".zip").delete();
                 connect();
-                testCheckAfterCrash();
+                controllerCheckAfterCrash();
             } catch (Exception e) {
                 File zip = new File(baseDir + "/haltSeq" + sequenceId + ".zip");
                 File zipId = new File(baseDir + "/haltSeq" + sequenceId + "-" + errorId + ".zip");
@@ -285,7 +271,7 @@ public abstract class TestHalt extends TestBase {
             traceOperation("disconnect");
             conn.close();
         } catch (Exception e) {
-            trace("disconnect", e);
+            traceOperation("disconnect", e);
         }
     }
 

h2/src/test/org/h2/test/synth/TestHaltApp.java

@@ -29,7 +29,15 @@ public class TestHaltApp extends TestHalt {
     public static void main(String[] args) throws Exception {
         SelfDestructor.startCountdown(60);
         baseDir = TestHalt.DIR;
-        new TestHaltApp().start(args);
+        TestHaltApp app = new TestHaltApp();
+        if (args.length == 0) {
+            app.controllerTest();
+        } else {
+            app.operations = Integer.parseInt(args[0]);
+            app.flags = Integer.parseInt(args[1]);
+            app.value = Integer.parseInt(args[2]);
+            app.processRunRandom();
+        }
     }
 
     private void execute(Statement stat, String sql) throws SQLException {
@@ -40,7 +48,7 @@ public class TestHaltApp extends TestHalt {
     /**
      * Initialize the database.
      */
-    protected void testInit() throws SQLException {
+    protected void controllerInit() throws SQLException {
         Statement stat = conn.createStatement();
         // stat.execute("CREATE TABLE TEST(ID IDENTITY, NAME VARCHAR(255))");
         for (int i = 0; i < 20; i++) {
@@ -57,7 +65,7 @@ public class TestHaltApp extends TestHalt {
     /**
      * Wait after the application has been started.
      */
-    protected void testWaitAfterAppStart() throws Exception {
+    protected void controllerWaitAfterAppStart() throws Exception {
         int sleep = 10 + random.nextInt(300);
         if ((flags & FLAG_NO_DELAY) == 0) {
             sleep += 1000;
@@ -71,22 +79,22 @@ public class TestHaltApp extends TestHalt {
      *
      * @throws SQLException  if the data is not consistent.
      */
-    protected void testCheckAfterCrash() throws SQLException {
+    protected void controllerCheckAfterCrash() throws SQLException {
         Statement stat = conn.createStatement();
         ResultSet rs = stat.executeQuery("SELECT COUNT(*) FROM TEST");
         rs.next();
         int count = rs.getInt(1);
         System.out.println("count: " + count);
-        if (count % 2 == 0) {
+        if (count % 2 != 0) {
             traceOperation("row count: " + count);
-            throw new SQLException("Unexpected odd row count");
+            throw new SQLException("Unexpected odd row count: " + count);
         }
     }
 
     /**
      * Initialize the application.
      */
-    protected void appStart() throws SQLException {
+    protected void processAppStart() throws SQLException {
         Statement stat = conn.createStatement();
         if ((flags & FLAG_NO_DELAY) != 0) {
             execute(stat, "SET WRITE_DELAY 0");
@@ -95,13 +103,13 @@ public class TestHaltApp extends TestHalt {
         ResultSet rs = stat.executeQuery("SELECT COUNT(*) FROM TEST");
         rs.next();
         rowCount = rs.getInt(1);
-        trace("rows: " + rowCount, null);
+        traceOperation("rows: " + rowCount, null);
     }
 
     /**
      * Run the application code.
      */
-    protected void appRun() throws SQLException {
+    protected void processAppRun() throws SQLException {
         conn.setAutoCommit(false);
         traceOperation("setAutoCommit false");
         int rows = 10000 + value;
@@ -146,11 +154,11 @@ public class TestHaltApp extends TestHalt {
                 rowCount -= uc;
             }
             traceOperation("rowCount " + rowCount);
-            trace("rows now: " + rowCount, null);
+            traceOperation("rows now: " + rowCount, null);
             if (rowCount % 2 == 0) {
                 traceOperation("commit " + rowCount);
                 conn.commit();
-                trace("committed: " + rowCount, null);
+                traceOperation("committed: " + rowCount, null);
             }
             if ((flags & FLAG_NO_DELAY) != 0) {
                 if (random.nextInt(10) == 0 && (rowCount % 2 == 0)) {