Merge pull request #1655 from katzyn/deleteOnExit

Do not use File.deleteOnExit()

h2/src/docsrc/html/changelog.html

@@ -21,6 +21,8 @@ Change Log
 
 <h2>Next Version (unreleased)</h2>
 <ul>
+<li>Issue #1654: OOM in TestMemoryUsage, in big mode
+</li>
 <li>Issue #1651: TIMESTAMP values near DST may be changed in MVStore database due to UTC-based PageStore format in some
 temporary storages
 </li>

h2/src/docsrc/html/roadmap.html

@@ -63,8 +63,6 @@ See also <a href="build.html#providing_patches">Providing Patches</a>.
 </li><li>PostgreSQL catalog: use BEFORE SELECT triggers instead of views over metadata tables.
 </li><li>Test very large databases and LOBs (up to 256 GB).
 </li><li>Store all temp files in the temp directory.
-</li><li>Don't use temp files, specially not deleteOnExit (bug 4513817: File.deleteOnExit consumes memory).
-    Also to allow opening client / server (remote) connections when using LOBs.
 </li><li>Make DDL (Data Definition) operations transactional.
 </li><li>Deferred integrity checking (DEFERRABLE INITIALLY DEFERRED).
 </li><li>Groovy Stored Procedures: http://groovy.codehaus.org/GSQL

h2/src/main/org/h2/command/dml/Delete.java

@@ -76,7 +76,6 @@ public class Delete extends Prepared {
         session.getUser().checkRight(table, Right.DELETE);
         table.fire(session, Trigger.DELETE, true);
         table.lock(session, true, false);
-        RowList rows = new RowList(session);
         int limitRows = -1;
         if (limitExpr != null) {
             Value v = limitExpr.getValue(session);
@@ -84,7 +83,7 @@ public class Delete extends Prepared {
                 limitRows = v.getInt();
             }
         }
-        try {
+        try (RowList rows = new RowList(session)) {
             setCurrentRowNumber(0);
             int count = 0;
             while (limitRows != 0 && targetTableFilter.next()) {
@@ -128,8 +127,6 @@ public class Delete extends Prepared {
             }
             table.fire(session, Trigger.DELETE, false);
             return count;
-        } finally {
-            rows.close();
         }
     }
 

h2/src/main/org/h2/command/dml/Update.java

@@ -102,8 +102,7 @@ public class Update extends Prepared {
     public int update() {
         targetTableFilter.startQuery(session);
         targetTableFilter.reset();
-        RowList rows = new RowList(session);
-        try {
+        try (RowList rows = new RowList(session)) {
             Table table = targetTableFilter.getTable();
             session.getUser().checkRight(table, Right.UPDATE);
             table.fire(session, Trigger.UPDATE, true);
@@ -207,8 +206,6 @@ public class Update extends Prepared {
             }
             table.fire(session, Trigger.UPDATE, false);
             return count;
-        } finally {
-            rows.close();
         }
     }
 

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

@@ -1410,12 +1410,27 @@ public class Database implements DataHandler {
      *            hook
      */
     void close(boolean fromShutdownHook) {
+        DbException b = backgroundException.getAndSet(null);
+        try {
+            closeImpl(fromShutdownHook);
+        } catch (Throwable t) {
+            if (b != null) {
+                t.addSuppressed(b);
+            }
+            throw t;
+        }
+        if (b != null) {
+            // wrap the exception, so we see it was thrown here
+            throw DbException.get(b.getErrorCode(), b, b.getMessage());
+        }
+    }
+
+    private void closeImpl(boolean fromShutdownHook) {
         try {
             synchronized (this) {
                 if (closing) {
                     return;
                 }
-                throwLastBackgroundException();
                 if (fileLockMethod == FileLockMethod.SERIALIZED &&
                         !reconnectChangePending) {
                     // another connection may have written something - don't write
@@ -1956,8 +1971,7 @@ public class Database implements DataHandler {
             if (!persistent) {
                 name = "memFS:" + name;
             }
-            return FileUtils.createTempFile(name,
-                    Constants.SUFFIX_TEMP_FILE, true, inTempDir);
+            return FileUtils.createTempFile(name, Constants.SUFFIX_TEMP_FILE, inTempDir);
         } catch (IOException e) {
             throw DbException.convertIOException(e, databaseName);
         }

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

@@ -376,7 +376,7 @@ public class SessionRemote extends SessionWithState implements DataHandler {
                 traceSystem.setLevelFile(level);
                 if (level > 0 && level < 4) {
                     String file = FileUtils.createTempFile(prefix,
-                            Constants.SUFFIX_TRACE_FILE, false, false);
+                            Constants.SUFFIX_TRACE_FILE, false);
                     traceSystem.setFileName(file);
                 }
             } catch (IOException e) {

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

@@ -143,6 +143,7 @@ public class UndoLog {
             if (file == null) {
                 String fileName = database.createTempFile();
                 file = database.openFile(fileName, "rw", false);
+                file.autoDelete();
                 file.setCheckedWriting(false);
                 file.setLength(FileStore.HEADER_LENGTH);
             }
@@ -160,7 +161,6 @@ public class UndoLog {
             storedEntries += records.size();
             memoryUndo = 0;
             records.clear();
-            file.autoDelete();
         }
     }
 

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

@@ -159,7 +159,7 @@ public abstract class MVTempResult implements ResultExternal {
      */
     MVTempResult(Database database, int columnCount, int visibleColumnCount) {
         try {
-            String fileName = FileUtils.createTempFile("h2tmp", Constants.SUFFIX_TEMP_FILE, false, true);
+            String fileName = FileUtils.createTempFile("h2tmp", Constants.SUFFIX_TEMP_FILE, true);
             Builder builder = new MVStore.Builder().fileName(fileName).cacheSize(0).autoCommitDisabled();
             byte[] key = database.getFileEncryptionKey();
             if (key != null) {

h2/src/main/org/h2/result/RowList.java

@@ -20,7 +20,7 @@ import org.h2.value.Value;
  * A list of rows. If the list grows too large, it is buffered to disk
  * automatically.
  */
-public class RowList {
+public class RowList implements AutoCloseable {
 
     private final Session session;
     private final ArrayList<Row> list = Utils.newSmallArrayList();
@@ -48,14 +48,14 @@ public class RowList {
     }
 
     private void writeRow(Data buff, Row r) {
-        buff.checkCapacity(1 + Data.LENGTH_INT * 8);
+        buff.checkCapacity(2 + Data.LENGTH_INT * 3 + Data.LENGTH_LONG);
         buff.writeByte((byte) 1);
         buff.writeInt(r.getMemory());
         int columnCount = r.getColumnCount();
         buff.writeInt(columnCount);
         buff.writeLong(r.getKey());
         buff.writeInt(r.getVersion());
-        buff.writeInt(r.isDeleted() ? 1 : 0);
+        buff.writeByte(r.isDeleted() ? (byte) 1 : (byte) 0);
         for (int i = 0; i < columnCount; i++) {
             Value v = r.getValue(i);
             buff.checkCapacity(1);
@@ -104,7 +104,6 @@ public class RowList {
             writeRow(buff, r);
         }
         flushBuffer(buff);
-        file.autoDelete();
         list.clear();
         memory = 0;
     }
@@ -169,7 +168,7 @@ public class RowList {
         int columnCount = buff.readInt();
         long key = buff.readLong();
         int version = buff.readInt();
-        boolean deleted = buff.readInt() == 1;
+        boolean deleted = buff.readByte() != 0;
         Value[] values = new Value[columnCount];
         for (int i = 0; i < columnCount; i++) {
             Value v;
@@ -242,9 +241,9 @@ public class RowList {
     /**
      * Close the result list and delete the temporary file.
      */
+    @Override
     public void close() {
         if (file != null) {
-            file.autoDelete();
             file.closeAndDeleteSilently();
             file = null;
             rowBuff = null;

h2/src/main/org/h2/store/Data.java

@@ -73,7 +73,7 @@ public class Data {
     /**
      * The length of a long value.
      */
-    private static final int LENGTH_LONG = 8;
+    public static final int LENGTH_LONG = 8;
 
     /**
      * Storage type for ValueRow.

h2/src/main/org/h2/store/fs/FilePath.java

@@ -250,14 +250,11 @@ public abstract class FilePath {
      * Create a new temporary file.
      *
      * @param suffix the suffix
-     * @param deleteOnExit if the file should be deleted when the virtual
-     *            machine exists
      * @param inTempDir if the file should be stored in the temporary directory
      * @return the name of the created file
      */
     @SuppressWarnings("unused")
-    public FilePath createTempFile(String suffix, boolean deleteOnExit,
-            boolean inTempDir) throws IOException {
+    public FilePath createTempFile(String suffix, boolean inTempDir) throws IOException {
         while (true) {
             FilePath p = getPath(name + getNextTempFileNamePart(false) + suffix);
             if (p.exists() || !p.createFile()) {

h2/src/main/org/h2/store/fs/FilePathDisk.java

@@ -381,8 +381,7 @@ public class FilePathDisk extends FilePath {
     }
 
     @Override
-    public FilePath createTempFile(String suffix, boolean deleteOnExit,
-            boolean inTempDir) throws IOException {
+    public FilePath createTempFile(String suffix, boolean inTempDir) throws IOException {
         String fileName = name + ".";
         String prefix = new File(fileName).getName();
         File dir;
@@ -399,15 +398,6 @@ public class FilePathDisk extends FilePath {
                 getNextTempFileNamePart(true);
                 continue;
             }
-            if (deleteOnExit) {
-                try {
-                    f.deleteOnExit();
-                } catch (Throwable e) {
-                    // sometimes this throws a NullPointerException
-                    // at java.io.DeleteOnExitHook.add(DeleteOnExitHook.java:33)
-                    // we can ignore it
-                }
-            }
             return get(f.getCanonicalPath());
         }
     }

h2/src/main/org/h2/store/fs/FilePathRec.java

@@ -46,11 +46,9 @@ public class FilePathRec extends FilePathWrapper {
     }
 
     @Override
-    public FilePath createTempFile(String suffix, boolean deleteOnExit,
-            boolean inTempDir) throws IOException {
-        log(Recorder.CREATE_TEMP_FILE, unwrap(name) + ":" + suffix + ":" +
-                deleteOnExit + ":" + inTempDir);
-        return super.createTempFile(suffix, deleteOnExit, inTempDir);
+    public FilePath createTempFile(String suffix, boolean inTempDir) throws IOException {
+        log(Recorder.CREATE_TEMP_FILE, unwrap(name) + ":" + suffix + ":" + inTempDir);
+        return super.createTempFile(suffix, inTempDir);
     }
 
     @Override

h2/src/main/org/h2/store/fs/FilePathWrapper.java

@@ -158,9 +158,8 @@ public abstract class FilePathWrapper extends FilePath {
     }
 
     @Override
-    public FilePath createTempFile(String suffix, boolean deleteOnExit,
-            boolean inTempDir) throws IOException {
-        return wrap(base.createTempFile(suffix, deleteOnExit, inTempDir));
+    public FilePath createTempFile(String suffix, boolean inTempDir) throws IOException {
+        return wrap(base.createTempFile(suffix, inTempDir));
     }
 
 }

h2/src/main/org/h2/store/fs/FilePathZip.java

@@ -234,13 +234,11 @@ public class FilePathZip extends FilePath {
     }
 
     @Override
-    public FilePath createTempFile(String suffix, boolean deleteOnExit,
-            boolean inTempDir) throws IOException {
+    public FilePath createTempFile(String suffix, boolean inTempDir) throws IOException {
         if (!inTempDir) {
             throw new IOException("File system is read-only");
         }
-        return new FilePathDisk().getPath(name).createTempFile(suffix,
-                deleteOnExit, true);
+        return new FilePathDisk().getPath(name).createTempFile(suffix, true);
     }
 
     @Override

h2/src/main/org/h2/store/fs/FileUtils.java

@@ -335,15 +335,12 @@ public class FileUtils {
      * @param prefix the prefix of the file name (including directory name if
      *            required)
      * @param suffix the suffix
-     * @param deleteOnExit if the file should be deleted when the virtual
-     *            machine exists
      * @param inTempDir if the file should be stored in the temporary directory
      * @return the name of the created file
      */
     public static String createTempFile(String prefix, String suffix,
-            boolean deleteOnExit, boolean inTempDir) throws IOException {
-        return FilePath.get(prefix).createTempFile(
-                suffix, deleteOnExit, inTempDir).toString();
+            boolean inTempDir) throws IOException {
+        return FilePath.get(prefix).createTempFile(suffix, inTempDir).toString();
     }
 
     /**

h2/src/main/org/h2/value/ValueLobDb.java

@@ -179,7 +179,7 @@ public class ValueLobDb extends Value {
         if (path.isEmpty()) {
             path = SysProperties.PREFIX_TEMP_FILE;
         }
-        return FileUtils.createTempFile(path, Constants.SUFFIX_TEMP_FILE, true, true);
+        return FileUtils.createTempFile(path, Constants.SUFFIX_TEMP_FILE, true);
     }
 
     /**

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

@@ -13,6 +13,7 @@ import java.sql.Statement;
 import java.util.Random;
 import java.util.concurrent.TimeUnit;
 
+import org.h2.api.ErrorCode;
 import org.h2.test.TestBase;
 import org.h2.test.TestDb;
 import org.h2.util.Utils;
@@ -72,7 +73,7 @@ public class TestMemoryUsage extends TestDb {
             }
         } finally {
             freeMemory();
-            conn.close();
+            closeConnection(conn);
         }
     }
 
@@ -145,7 +146,24 @@ public class TestMemoryUsage extends TestDb {
             }
         } finally {
             freeMemory();
+            closeConnection(conn);
+        }
+    }
+
+    /**
+     * Closes the specified connection. It silently consumes OUT_OF_MEMORY that
+     * may happen in background thread during the tests.
+     *
+     * @param conn connection to close
+     * @throws SQLException on other SQL exception
+     */
+    private static void closeConnection(Connection conn) throws SQLException {
+        try {
             conn.close();
+        } catch (SQLException e) {
+            if (e.getErrorCode() != ErrorCode.OUT_OF_MEMORY) {
+                throw e;
+            }
         }
     }
 

h2/src/test/org/h2/test/unit/TestFileSystem.java

@@ -370,7 +370,7 @@ public class TestFileSystem extends TestDb {
         new AssertThrows(IOException.class) {
             @Override
             public void test() throws IOException {
-                FileUtils.createTempFile(f, ".tmp", false, false);
+                FileUtils.createTempFile(f, ".tmp", false);
         }};
         final FileChannel channel = FileUtils.open(f, "r");
         new AssertThrows(IOException.class) {
@@ -670,7 +670,7 @@ public class TestFileSystem extends TestDb {
 
     private void testRandomAccess(String fsBase, int seed) throws Exception {
         StringBuilder buff = new StringBuilder();
-        String s = FileUtils.createTempFile(fsBase + "/tmp", ".tmp", false, false);
+        String s = FileUtils.createTempFile(fsBase + "/tmp", ".tmp", false);
         File file = new File(TestBase.BASE_TEST_DIR + "/tmp");
         file.getParentFile().mkdirs();
         file.delete();
@@ -784,7 +784,7 @@ public class TestFileSystem extends TestDb {
 
     private void testTempFile(String fsBase) throws Exception {
         int len = 10000;
-        String s = FileUtils.createTempFile(fsBase + "/tmp", ".tmp", false, false);
+        String s = FileUtils.createTempFile(fsBase + "/tmp", ".tmp", false);
         OutputStream out = FileUtils.newOutputStream(s, false);
         byte[] buffer = new byte[len];
         out.write(buffer);
@@ -804,7 +804,7 @@ public class TestFileSystem extends TestDb {
     }
 
     private void testConcurrent(String fsBase) throws Exception {
-        String s = FileUtils.createTempFile(fsBase + "/tmp", ".tmp", false, false);
+        String s = FileUtils.createTempFile(fsBase + "/tmp", ".tmp", false);
         File file = new File(TestBase.BASE_TEST_DIR + "/tmp");
         file.getParentFile().mkdirs();
         file.delete();

h2/src/test/org/h2/test/utils/FilePathDebug.java

@@ -191,10 +191,9 @@ public class FilePathDebug extends FilePathWrapper {
     }
 
     @Override
-    public FilePath createTempFile(String suffix, boolean deleteOnExit,
-            boolean inTempDir) throws IOException {
-        trace(name, "createTempFile", suffix, deleteOnExit, inTempDir);
-        return super.createTempFile(suffix, deleteOnExit, inTempDir);
+    public FilePath createTempFile(String suffix, boolean inTempDir) throws IOException {
+        trace(name, "createTempFile", suffix, inTempDir);
+        return super.createTempFile(suffix, inTempDir);
     }
 
     /**

h2/src/test/org/h2/test/utils/FilePathUnstable.java

@@ -198,9 +198,8 @@ public class FilePathUnstable extends FilePathWrapper {
     }
 
     @Override
-    public FilePath createTempFile(String suffix, boolean deleteOnExit,
-            boolean inTempDir) throws IOException {
-        return super.createTempFile(suffix, deleteOnExit, inTempDir);
+    public FilePath createTempFile(String suffix, boolean inTempDir) throws IOException {
+        return super.createTempFile(suffix, inTempDir);
     }
 
     @Override

h2/src/tools/org/h2/build/doc/dictionary.txt

@@ -805,4 +805,4 @@ queryparser tokenized freeze factorings recompilation unenclosed rfe dsync
 econd irst bcef ordinality nord unnest
 analyst occupation distributive josaph aor engineer sajeewa isuru randil kevin doctor businessman artist ashan
 corrupts splitted disruption unintentional octets preconditions predicates subq objectweb insn opcodes
-preserves masking holder unboxing avert iae transformed
+preserves masking holder unboxing avert iae transformed subtle

h2/src/tools/org/h2/dev/fs/FilePathZip2.java

@@ -61,13 +61,11 @@ public class FilePathZip2 extends FilePath {
     }
 
     @Override
-    public FilePath createTempFile(String suffix, boolean deleteOnExit,
-            boolean inTempDir) throws IOException {
+    public FilePath createTempFile(String suffix, boolean inTempDir) throws IOException {
         if (!inTempDir) {
             throw new IOException("File system is read-only");
         }
-        return new FilePathDisk().getPath(name).createTempFile(suffix,
-                deleteOnExit, true);
+        return new FilePathDisk().getPath(name).createTempFile(suffix, true);
     }
 
     @Override