diff --git a/api/src/org/labkey/api/data/CachedResultSet.java b/api/src/org/labkey/api/data/CachedResultSet.java index 273e9cd40a4..1e03687a288 100644 --- a/api/src/org/labkey/api/data/CachedResultSet.java +++ b/api/src/org/labkey/api/data/CachedResultSet.java @@ -17,6 +17,7 @@ package org.labkey.api.data; import org.apache.commons.beanutils.ConvertUtils; +import java.lang.ref.Cleaner; import org.apache.commons.collections4.IteratorUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -25,7 +26,6 @@ import org.labkey.api.collections.RowMap; import org.labkey.api.dataiterator.DataIterator; import org.labkey.api.miniprofiler.MiniProfiler; -import org.labkey.api.settings.AppProps; import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.MemTracker; import org.labkey.api.util.ResultSetUtil; @@ -77,13 +77,8 @@ public class CachedResultSet implements ResultSet, TableResultSet // data private final ArrayList> _rowMaps; private final boolean _isComplete; - @Nullable - private final StackTraceElement[] _stackTrace; - private final String _threadName; private boolean _wasClosed = false; - private boolean _requireClose = true; - private String _url = null; // state private int _row = -1; @@ -91,13 +86,69 @@ public class CachedResultSet implements ResultSet, TableResultSet private int _fetchSize = 1; private Object _lastObject = null; + private static final Cleaner CLEANER = Cleaner.create(); + + private static class CachedResultSetState implements Runnable + { + private final boolean _requireClose; + private final @Nullable StackTraceElement[] _stackTrace; + private final String _threadName; + private final String _url; + private final Logger _log; + + private boolean _wasClosed = false; + + private CachedResultSetState(boolean requireClose, @Nullable StackTraceElement[] stackTrace, String threadName, String url, Logger log) + { + _requireClose = requireClose; + _stackTrace = stackTrace; + _threadName = threadName; + _url = url; + _log = log; + } + + @Override + public void run() + { + if (!_wasClosed) + { + if (_requireClose) + { + StringBuilder error = new StringBuilder("CachedResultSet was not closed."); + if (null != _threadName) + { + error.append(" Created by thread ").append(_threadName); + } + if (null != _url) + { + error.append(" for URL ").append(_url); + } + if (null != _stackTrace) + { + error.append("\n").append(ExceptionUtil.renderStackTrace(_stackTrace)); + } + _log.error(error); + } + _wasClosed = true; + } + } + + private void close() + { + _wasClosed = true; + } + } + + private final CachedResultSetState _state; + private final Cleaner.Cleanable _cleanable; + /* Constructor is not normally used... see CachedResultSets for static factory methods. stackTrace is used to set an alternate stack trace -- good for async queries, to indicate original creation stack trace */ - CachedResultSet(ResultSetMetaData md, ArrayList> maps, boolean isComplete, @Nullable StackTraceElement[] stackTrace) + CachedResultSet(ResultSetMetaData md, ArrayList> maps, boolean isComplete, boolean requireClose, @Nullable StackTraceElement[] stackTrace) { _rowMaps = maps; _isComplete = isComplete; @@ -120,25 +171,23 @@ public class CachedResultSet implements ResultSet, TableResultSet throw new RuntimeSQLException(x); } - if (MiniProfiler.isCollectTroubleshootingStackTraces()) + String url = null; + String threadName = null; + if (MiniProfiler.isCollectTroubleshootingStackTraces() || true) // TODO - remove hack! { // Stash stack trace that created this CachedRowSet - if (null != stackTrace) + if (null == stackTrace) { - _stackTrace = stackTrace; - } - else - { - _stackTrace = MiniProfiler.getTroubleshootingStackTrace(); + stackTrace = MiniProfiler.getTroubleshootingStackTrace(); } - _threadName = Thread.currentThread().getName(); + threadName = Thread.currentThread().getName(); if (HttpView.getStackSize() > 0) { try { - _url = ViewServlet.getOriginalURL(); + url = ViewServlet.getOriginalURL(); } catch (Exception x) { @@ -146,24 +195,11 @@ public class CachedResultSet implements ResultSet, TableResultSet } } } - else - { - _stackTrace = null; - _threadName = null; - } - MemTracker.getInstance().put(this); - } - - public boolean isRequireClose() - { - return _requireClose; - } + _state = new CachedResultSetState(requireClose, stackTrace, threadName, url, _log); + _cleanable = CLEANER.register(this, _state); - public CachedResultSet setRequireClose(boolean requireClose) - { - _requireClose = requireClose; - return this; + MemTracker.getInstance().put(this); } @Override @@ -223,6 +259,8 @@ public boolean next() public void close() { _wasClosed = true; + _state.close(); + _cleanable.clean(); } @Override @@ -729,28 +767,6 @@ public void afterLast() _row = _rowMaps.size(); } - @Override - protected void finalize() throws Throwable - { - if (!_wasClosed) - { - close(); - - if (_requireClose && AppProps.getInstance().isDevMode()) - { - StringBuilder error = new StringBuilder("CachedResultSet was not closed."); - if (null != _url) - error.append("\nURL: ").append(_url); - else if (_threadName != null) - error.append("\nthreadName: ").append(_threadName); - error.append("\nStack trace from the creation:"); - error.append(ExceptionUtil.renderStackTrace(_stackTrace)); - - _log.error(error); - } - } - super.finalize(); - } @Override public boolean first() diff --git a/api/src/org/labkey/api/data/CachedResultSets.java b/api/src/org/labkey/api/data/CachedResultSets.java index 4cf3f7e6b6f..67ac2fec66e 100644 --- a/api/src/org/labkey/api/data/CachedResultSets.java +++ b/api/src/org/labkey/api/data/CachedResultSets.java @@ -32,12 +32,12 @@ */ public class CachedResultSets { - public static CachedResultSet create(ResultSet rs, boolean cacheMetaData, int maxRows) throws SQLException + public static CachedResultSet create(ResultSet rs, boolean cacheMetaData, boolean requireClose, int maxRows) throws SQLException { - return create(rs, cacheMetaData, maxRows, null, QueryLogging.emptyQueryLogging()); + return create(rs, cacheMetaData, requireClose, maxRows, null, QueryLogging.emptyQueryLogging()); } - public static CachedResultSet create(ResultSet rsIn, boolean cacheMetaData, int maxRows, @Nullable StackTraceElement[] stackTrace, QueryLogging queryLogging) throws SQLException + public static CachedResultSet create(ResultSet rsIn, boolean cacheMetaData, boolean requireClose, int maxRows, @Nullable StackTraceElement[] stackTrace, QueryLogging queryLogging) throws SQLException { try (ResultSet rs = new LoggingResultSetWrapper(rsIn, queryLogging)) // TODO: avoid if we're passed a read-only and empty one?? { @@ -58,13 +58,13 @@ public static CachedResultSet create(ResultSet rsIn, boolean cacheMetaData, int // If we have another row, then we're not complete boolean isComplete = !rs.next(); - return new CachedResultSet(md, list, isComplete, stackTrace); + return new CachedResultSet(md, list, isComplete, requireClose, stackTrace); } } public static CachedResultSet create(ResultSetMetaData md, List> maps, boolean isComplete) { - return new CachedResultSet(md, convertToRowMaps(md, maps), isComplete, null); + return new CachedResultSet(md, convertToRowMaps(md, maps), isComplete, true, null); } public static CachedResultSet create(List> maps) @@ -88,7 +88,7 @@ public static CachedResultSet create(List> maps, Collection< ResultSetMetaData md = createMetaData(columnNames); // Avoid error message from CachedResultSet.finalize() about unclosed CachedResultSet. - try (CachedResultSet crs = new CachedResultSet(md, convertToRowMaps(md, maps), true, null)) + try (CachedResultSet crs = new CachedResultSet(md, convertToRowMaps(md, maps), true, true, null)) { return crs; } diff --git a/api/src/org/labkey/api/data/ConnectionWrapper.java b/api/src/org/labkey/api/data/ConnectionWrapper.java index bf81461c15f..94b46667dae 100644 --- a/api/src/org/labkey/api/data/ConnectionWrapper.java +++ b/api/src/org/labkey/api/data/ConnectionWrapper.java @@ -17,6 +17,7 @@ package org.labkey.api.data; import org.apache.commons.collections4.multimap.HashSetValuedHashMap; +import java.lang.ref.Cleaner; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.LoggerContext; @@ -78,6 +79,59 @@ public class ConnectionWrapper implements java.sql.Connection private static final Logger packageLogger = LogManager.getLogger(ConnectionWrapper.class.getPackageName()); private static final Logger LOG = LogHelper.getLogger(ConnectionWrapper.class, "All JDBC metadata and SQL execution calls being made"); + private static final Cleaner CLEANER = Cleaner.create(); + + private static class ConnectionState implements Runnable + { + private Connection _connection; + private final DbScope _scope; + private final ConnectionWrapper _wrapper; // This is risky, but we need toString() + private final Logger _log; + + private boolean _closed = false; + + private ConnectionState(Connection connection, DbScope scope, ConnectionWrapper wrapper, Logger log) + { + _connection = connection; + _scope = scope; + _wrapper = wrapper; + _log = log; + } + + @Override + public void run() + { + if (_connection != null && !_closed) + { + _log.error("Connection was not closed! " + _wrapper.toString()); + realClose(); + } + } + + private void realClose() + { + if (!_closed) + { + try + { + _wrapper.realCloseInternal(); + } + catch (SQLException e) + { + _log.error("Failed to close connection", e); + } + finally + { + _closed = true; + _connection = null; + } + } + } + } + + private final ConnectionState _state; + private final Cleaner.Cleanable _cleanable; + private static final Set _openConnections = Collections.synchronizedSet(new HashSet<>()); private static final Set _loggedLeaks = new HashSet<>(); private static final boolean _explicitLogger = initializeExplicitLogger(); @@ -165,6 +219,8 @@ public ConnectionWrapper(Connection conn, DbScope scope, Integer spid, Connectio _openConnections.add(this); _log = log != null ? log : getConnectionLogger(); + _state = new ConnectionState(_connection, _scope, this, _log); + _cleanable = CLEANER.register(this, _state); } /** this is a best guess logger, pass one in to be predictable */ @@ -457,7 +513,13 @@ void internalClose() throws SQLException _type.close(_scope, this, this::realClose); } - private void realClose() throws SQLException + private void realClose() + { + _state.realClose(); + _cleanable.clean(); + } + + private void realCloseInternal() throws SQLException { _openConnections.remove(this); _loggedLeaks.remove(this); @@ -938,18 +1000,6 @@ public boolean isWrapperFor(Class iface) return iface.isAssignableFrom(_connection.getClass()); } - @Override - protected void finalize() throws Throwable - { - // If the thread was banned from getting a connection, _connection will be null, and we shouldn't complain that it wasn't closed - if (_connection != null && !isClosed()) - { - LOG.error("Connection was not closed! " + this); - realClose(); - } - - super.finalize(); - } public @NotNull Closer getRunOnClose() { diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index 14eca295651..dcd387315b6 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -1878,7 +1878,7 @@ private static void detectUnexpectedConnections(Connection conn, LabKeyDataSourc stmt.setString(1, databaseName); stmt.setString(2, applicationName); - try (CachedResultSet rs = CachedResultSets.create(stmt.executeQuery(), true, 1000)) + try (CachedResultSet rs = CachedResultSets.create(stmt.executeQuery(), true, true, 1000)) { count = rs.getSize(); if (count != 0) diff --git a/api/src/org/labkey/api/data/ParameterMapStatement.java b/api/src/org/labkey/api/data/ParameterMapStatement.java index 8c6bdab4ed0..a1472e98ab8 100644 --- a/api/src/org/labkey/api/data/ParameterMapStatement.java +++ b/api/src/org/labkey/api/data/ParameterMapStatement.java @@ -1,6 +1,7 @@ package org.labkey.api.data; import org.apache.logging.log4j.Level; +import java.lang.ref.Cleaner; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.arrays.IntegerArray; @@ -52,6 +53,26 @@ public class ParameterMapStatement implements AutoCloseable private ExceptionFramework _exceptionFramework = ExceptionFramework.Spring; + private static final Cleaner CLEANER = Cleaner.create(); + + private static class ParameterMapState implements Runnable + { + private Runnable _onClose; + + @Override + public void run() + { + if (null != _onClose) + { + _onClose.run(); + _onClose = null; + } + } + } + + private final ParameterMapState _state = new ParameterMapState(); + private final Cleaner.Cleanable _cleanable = CLEANER.register(this, _state); + protected ParameterMapStatement() { //for testing subclasses (see NoopParameterMap) @@ -519,6 +540,7 @@ public void onClose(Runnable r) if (null != _onClose) throw new IllegalStateException("only one onClose() callback supported"); _onClose = r; + _state._onClose = r; } @@ -532,17 +554,11 @@ protected void afterClose() _onClose.run(); } _onClose = null; + _state._onClose = null; + _cleanable.clean(); } - @Override - protected void finalize() throws Throwable - { - super.finalize(); - assert null == _onClose; - if (null != _onClose) - _onClose.run(); - } String _debugSql; diff --git a/api/src/org/labkey/api/data/ResultSetImpl.java b/api/src/org/labkey/api/data/ResultSetImpl.java index 7d26efc4acb..96cf846bb46 100644 --- a/api/src/org/labkey/api/data/ResultSetImpl.java +++ b/api/src/org/labkey/api/data/ResultSetImpl.java @@ -17,6 +17,7 @@ package org.labkey.api.data; import org.apache.logging.log4j.LogManager; +import java.lang.ref.Cleaner; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -55,6 +56,73 @@ public class ResultSetImpl extends LoggingResultSetWrapper implements TableResul protected boolean _wasClosed = false; + private static final Cleaner CLEANER = Cleaner.create(); + + private static class ResultSetState implements Runnable + { + private final String _creatingThreadName; + private final StackTraceElement[] _debugCreated; + private final @Nullable DbScope _scope; + private final @Nullable Connection _connection; + private final ResultSet _rs; // This is the underlying result set + + private boolean _wasClosed = false; + + private ResultSetState(String creatingThreadName, StackTraceElement[] debugCreated, @Nullable DbScope scope, @Nullable Connection connection, ResultSet rs) + { + _creatingThreadName = creatingThreadName; + _debugCreated = debugCreated; + _scope = scope; + _connection = connection; + _rs = rs; + } + + @Override + public void run() + { + if (!_wasClosed) + { + _log.error("ResultSet was not closed. Created by thread " + _creatingThreadName + " with stacktrace: " + ExceptionUtil.renderStackTrace(_debugCreated)); + close(); + } + } + + private void close() + { + if (!_wasClosed) + { + try + { + if (null != _scope) + { + Statement stmt = _rs.getStatement(); + _rs.close(); + if (stmt != null) + { + stmt.close(); + } + _scope.releaseConnection(_connection); + } + else + { + _rs.close(); + } + } + catch (SQLException e) + { + _log.error("Error closing ResultSet", e); + } + finally + { + _wasClosed = true; + } + } + } + } + + private final ResultSetState _state; + private final Cleaner.Cleanable _cleanable; + public ResultSetImpl(ResultSet rs, QueryLogging queryLogging) { this(rs, Table.ALL_ROWS, queryLogging); @@ -95,6 +163,8 @@ else if (rs.getStatement() != null) throw new RuntimeSQLException(e); } _scope = scope; + _state = new ResultSetState(_creatingThreadName, _debugCreated, _scope, _connection, rs); + _cleanable = CLEANER.register(this, _state); } @@ -176,20 +246,8 @@ public void close() throws SQLException } else { - // Uncached case... close everything down - if (null != _scope) - { - Statement stmt = getStatement(); - super.close(); - if (stmt != null) - { - stmt.close(); - } - _scope.releaseConnection(_connection); - } - else - super.close(); - + _state.close(); + _cleanable.clean(); _wasClosed = true; } } @@ -213,16 +271,6 @@ public Map getRowMap() } - @Override - protected void finalize() throws Throwable - { - if (!_wasClosed) - { - close(); - _log.error("ResultSet was not closed. Created by thread " + _creatingThreadName + " with stacktrace: " + ExceptionUtil.renderStackTrace(_debugCreated)); - } - super.finalize(); - } @Override public @Nullable Connection getConnection() diff --git a/api/src/org/labkey/api/data/ResultSetSelector.java b/api/src/org/labkey/api/data/ResultSetSelector.java index 0ff8e659890..1b83764d5bf 100644 --- a/api/src/org/labkey/api/data/ResultSetSelector.java +++ b/api/src/org/labkey/api/data/ResultSetSelector.java @@ -103,7 +103,7 @@ protected TableResultSet wrapResultSet(ResultSet rs, Connection conn, boolean ca { if (cache) { - return CachedResultSets.create(rs, true, Table.ALL_ROWS).setRequireClose(requireClose); + return CachedResultSets.create(rs, true, requireClose, Table.ALL_ROWS); } else { diff --git a/api/src/org/labkey/api/data/ResultSetSelectorTestCase.java b/api/src/org/labkey/api/data/ResultSetSelectorTestCase.java index 3d3becd7b29..bdcaa86239c 100644 --- a/api/src/org/labkey/api/data/ResultSetSelectorTestCase.java +++ b/api/src/org/labkey/api/data/ResultSetSelectorTestCase.java @@ -79,7 +79,7 @@ private void test(DbScope scope, ResultSet rs, Class clazz) throws SQLExc assertEquals("Non-scrollable ResultSet can't be used with ScrollToTop", e.getMessage()); } - rs = CachedResultSets.create(rs, true, Table.ALL_ROWS, null, QueryLogging.emptyQueryLogging()); + rs = CachedResultSets.create(rs, true, true, Table.ALL_ROWS, null, QueryLogging.emptyQueryLogging()); } ResultSetSelector selector = new ResultSetSelector(scope, rs); diff --git a/api/src/org/labkey/api/data/SqlExecutingSelector.java b/api/src/org/labkey/api/data/SqlExecutingSelector.java index 71245935a1f..5f564a5fc10 100644 --- a/api/src/org/labkey/api/data/SqlExecutingSelector.java +++ b/api/src/org/labkey/api/data/SqlExecutingSelector.java @@ -211,7 +211,7 @@ protected TableResultSet wrapResultSet(ResultSet rs, Connection conn, boolean ca if (cache) { // Cache ResultSet and meta data - return CachedResultSets.create(rs, true, _maxRows, _loggingStacktrace, getQueryLogging()).setRequireClose(requireClose); + return CachedResultSets.create(rs, true, requireClose, _maxRows, _loggingStacktrace, getQueryLogging()); } else { diff --git a/api/src/org/labkey/api/files/FileSystemWatcherImpl.java b/api/src/org/labkey/api/files/FileSystemWatcherImpl.java index 3d1781b7ac1..5ba41e34bef 100644 --- a/api/src/org/labkey/api/files/FileSystemWatcherImpl.java +++ b/api/src/org/labkey/api/files/FileSystemWatcherImpl.java @@ -23,7 +23,6 @@ import org.jetbrains.annotations.Nullable; import org.junit.Assert; import org.junit.Test; -import org.labkey.api.annotations.JavaRuntimeVersion; import org.labkey.api.cloud.CloudWatchService; import org.labkey.api.cloud.CloudWatcherConfig; import org.labkey.api.collections.ConcurrentHashSet; @@ -591,10 +590,7 @@ public void overflow() assertEquals(3, created.size()); assertTrue(created.containsAll(Set.of("a", "b", "c"))); - // Note: In Java 17 on Windows, modified events occur on delete. This has never been the case on Linux and - // is no longer the case in Java 25. TODO: Delete this check once we require Java 25 - @JavaRuntimeVersion - Set expectedModified = SystemUtils.IS_OS_WINDOWS && SystemUtils.IS_JAVA_17 ? Set.of("a", "b", "c") : Set.of("a", "c"); + Set expectedModified = Set.of("a", "c"); assertEquals(expectedModified.size(), modified.size()); assertTrue(created.containsAll(expectedModified)); int deletedCount = deleted.size(); diff --git a/api/src/org/labkey/api/jsp/RecompilingJspClassLoader.java b/api/src/org/labkey/api/jsp/RecompilingJspClassLoader.java index 65f6164def5..4ec84fc0742 100644 --- a/api/src/org/labkey/api/jsp/RecompilingJspClassLoader.java +++ b/api/src/org/labkey/api/jsp/RecompilingJspClassLoader.java @@ -137,8 +137,8 @@ protected TldScanner newTldScanner(JspCServletContext context, boolean namespace jasper.setUriroot(jspJavaFileBuildDirectory.getParentFile().getParent() + "/jspWebappDir/webapp/"); jasper.setOutputDir(jspJavaFileBuildDirectory.getAbsolutePath()); jasper.setPackage("org.labkey.jsp.compiled"); - jasper.setCompilerTargetVM("17"); - jasper.setCompilerSourceVM("17"); + jasper.setCompilerTargetVM("25"); + jasper.setCompilerSourceVM("25"); jasper.setCompile(false); jasper.setListErrors(true); diff --git a/api/src/org/labkey/api/miniprofiler/MiniProfiler.java b/api/src/org/labkey/api/miniprofiler/MiniProfiler.java index 6c2c2fe6092..cf0ed2b4f92 100644 --- a/api/src/org/labkey/api/miniprofiler/MiniProfiler.java +++ b/api/src/org/labkey/api/miniprofiler/MiniProfiler.java @@ -269,7 +269,7 @@ public static boolean isCollectTroubleshootingStackTraces() @Nullable public static StackTraceElement[] getTroubleshootingStackTrace() { - if (isCollectTroubleshootingStackTraces()) + if (isCollectTroubleshootingStackTraces() || true) // TODO - remove hack { StackTraceElement[] fullStack = Thread.currentThread().getStackTrace(); if (fullStack.length > 0) diff --git a/api/src/org/labkey/api/module/JavaVersion.java b/api/src/org/labkey/api/module/JavaVersion.java index cf49b4d4608..6ca16166b5e 100644 --- a/api/src/org/labkey/api/module/JavaVersion.java +++ b/api/src/org/labkey/api/module/JavaVersion.java @@ -31,7 +31,6 @@ public enum JavaVersion { JAVA_UNSUPPORTED(-1, true, false, null), - JAVA_17(17, true, true, "https://docs.oracle.com/en/java/javase/17/docs/api/java.base/"), JAVA_25(25, false, true, "https://docs.oracle.com/en/java/javase/25/docs/api/java.base/"), JAVA_FUTURE(Integer.MAX_VALUE, false, false, "https://docs.oracle.com/en/java/javase/25/docs/api/java.base/"); @@ -132,6 +131,7 @@ public void test() test(15, JAVA_UNSUPPORTED); test(16, JAVA_UNSUPPORTED); + test(17, JAVA_UNSUPPORTED); test(18, JAVA_UNSUPPORTED); test(19, JAVA_UNSUPPORTED); test(20, JAVA_UNSUPPORTED); @@ -141,7 +141,6 @@ public void test() test(24, JAVA_UNSUPPORTED); // Good - test(17, JAVA_17); test(25, JAVA_25); // Future diff --git a/api/src/org/labkey/api/reader/DataLoader.java b/api/src/org/labkey/api/reader/DataLoader.java index 2250ca156d4..fa3df2b0052 100644 --- a/api/src/org/labkey/api/reader/DataLoader.java +++ b/api/src/org/labkey/api/reader/DataLoader.java @@ -31,7 +31,6 @@ import org.labkey.api.data.BaseColumnInfo; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; -import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.ImportAliasable; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MvUtil; @@ -44,7 +43,6 @@ import org.labkey.api.dataiterator.ScrollableDataIterator; import org.labkey.api.exp.MvColumn; import org.labkey.api.exp.MvFieldWrapper; -import org.labkey.api.exp.PropertyType; import org.labkey.api.iterator.CloseableFilteredIterator; import org.labkey.api.iterator.CloseableIterator; import org.labkey.api.query.BatchValidationException; @@ -88,7 +86,7 @@ public static DataLoaderService get() * We'll try each one in turn, falling back * to the more general as necessary **/ - private final static Class[] CONVERT_CLASSES = new Class[] + private final static Class[] CONVERT_CLASSES = new Class[] { Date.class, Integer.class, @@ -328,7 +326,7 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws int inferStartLine = _skipLines == -1 ? 1 : _skipLines; for (int f = 0; f < nCols; f++) { - List classesToTest = new ArrayList<>(Arrays.asList(CONVERT_CLASSES)); + List> classesToTest = new ArrayList<>(Arrays.asList(CONVERT_CLASSES)); int classIndex = -1; //NOTE: this means we have a header row @@ -343,8 +341,8 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws if (knownColumn != null) { - Class knownColumnClass = knownColumn.getJavaClass(); - classesToTest.add(0, knownColumnClass); + Class knownColumnClass = knownColumn.getJavaClass(); + classesToTest.addFirst(knownColumnClass); } } } diff --git a/api/src/org/labkey/api/reader/FastaLoader.java b/api/src/org/labkey/api/reader/FastaLoader.java index 34846511613..0e770d45f4a 100644 --- a/api/src/org/labkey/api/reader/FastaLoader.java +++ b/api/src/org/labkey/api/reader/FastaLoader.java @@ -18,6 +18,7 @@ import org.labkey.vfs.FileLike; import java.io.BufferedReader; +import java.lang.ref.Cleaner; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.Iterator; @@ -53,10 +54,33 @@ public interface FastaIteratorElementFactory U createNext(String header, byte[] body); } + private static final Cleaner CLEANER = Cleaner.create(); + + private static class FastaIteratorState implements Runnable + { + private BufferedReader _reader; + + @Override + public void run() + { + if (null != _reader) + { + try + { + _reader.close(); + } + catch (IOException ignored) {} + _reader = null; + } + } + } + public class FastaIterator implements Iterator { + private final FastaIteratorState _state = new FastaIteratorState(); + private final Cleaner.Cleanable _cleanable = CLEANER.register(this, _state); + private String _header = null; - private BufferedReader _reader = null; // Various measures of where we are in the file to help with progress and error reporting. private boolean _beforeFirst = true; @@ -72,7 +96,7 @@ private void init() try { // Detect Charset encoding based on BOM - _reader = Readers.getBOMDetectingReader(_fastaFile.getName().toLowerCase().endsWith(".gz") ? new GZIPInputStream(_fastaFile.openInputStream()): _fastaFile.openInputStream()); + _state._reader = Readers.getBOMDetectingReader(_fastaFile.getName().toLowerCase().endsWith(".gz") ? new GZIPInputStream(_fastaFile.openInputStream()): _fastaFile.openInputStream()); String line = getLine(); @@ -84,22 +108,14 @@ private void init() } else { - if (null != _reader) - _reader.close(); + _state.run(); throw new IllegalArgumentException("Invalid FASTA file. The file did not start with \">\"."); } } catch (IOException x) { - if (null != _reader) - { - try - { - _reader.close(); - } - catch (IOException ignored) {} - } + _state.run(); } _beforeFirst = false; @@ -108,7 +124,7 @@ private void init() private String getLine() throws IOException { - String line = _reader.readLine(); + String line = _state._reader.readLine(); if (null != line) { @@ -134,18 +150,6 @@ public boolean hasNext() return null != _header; } - /** - * Closes file just in case. - * @throws java.io.IOException if file is not closeable - */ - @Override - protected void finalize() throws Throwable - { - super.finalize(); //If iteration is not complete, still close the file... - if (null != _reader) - _reader.close(); - } - /** * Get next entry in file. * @return T or null if end of file @@ -219,14 +223,8 @@ public void remove() */ public void close() { - if (null != _reader) - try - { - _reader.close(); - } - catch (IOException ignored) {} - - _reader = null; + _state.run(); + _cleanable.clean(); _header = null; } diff --git a/api/src/org/labkey/api/reports/report/r/RConnectionHolder.java b/api/src/org/labkey/api/reports/report/r/RConnectionHolder.java index db2cbb90ba9..b80b24be73d 100644 --- a/api/src/org/labkey/api/reports/report/r/RConnectionHolder.java +++ b/api/src/org/labkey/api/reports/report/r/RConnectionHolder.java @@ -16,6 +16,7 @@ package org.labkey.api.reports.report.r; import org.rosuda.REngine.Rserve.RConnection; +import java.lang.ref.Cleaner; import javax.script.ScriptException; import jakarta.servlet.http.HttpSessionBindingEvent; @@ -27,6 +28,29 @@ // public class RConnectionHolder implements HttpSessionBindingListener { + private static final Cleaner CLEANER = Cleaner.create(); + + private static class RConnectionState implements Runnable + { + private RConnection _connection; + + @Override + public void run() + { + if (_connection != null) + { + if (_connection.isConnected()) + { + _connection.close(); + } + _connection = null; + } + } + } + + private final RConnectionState _state = new RConnectionState(); + private final Cleaner.Cleanable _cleanable = CLEANER.register(this, _state); + RConnection _connection; boolean _inUse; Object _clientContext; // client supplied value to help identify a report session @@ -45,11 +69,6 @@ public RConnectionHolder(String reportSessionId, Object clientContext) } } - @Override - protected void finalize() - { - close(); - } public static HashSet getReportSessions() { @@ -84,6 +103,7 @@ public void setConnection(RConnection connection) { close(); _connection = connection; + _state._connection = connection; } } @@ -136,13 +156,8 @@ public void setInUse(boolean value) private void close() { - if (_connection != null) - { - if (_connection.isConnected()) - { - _connection.close(); - } - _connection = null; - } + _state.run(); + _cleanable.clean(); + _connection = null; } } \ No newline at end of file diff --git a/api/src/org/labkey/api/util/MultiPhaseCPUTimer.java b/api/src/org/labkey/api/util/MultiPhaseCPUTimer.java index a787052fff7..ef5679b37a5 100644 --- a/api/src/org/labkey/api/util/MultiPhaseCPUTimer.java +++ b/api/src/org/labkey/api/util/MultiPhaseCPUTimer.java @@ -203,11 +203,5 @@ public String getTimings(String prefix, Order order, String deliminator) return sb.toString(); } - @Override - protected void finalize() throws Throwable - { - assert _closed && null == _currentPhase; - super.finalize(); - } } } diff --git a/experiment/src/org/labkey/experiment/ExperimentRunGraph.java b/experiment/src/org/labkey/experiment/ExperimentRunGraph.java index f96388a9927..4f7b8265315 100644 --- a/experiment/src/org/labkey/experiment/ExperimentRunGraph.java +++ b/experiment/src/org/labkey/experiment/ExperimentRunGraph.java @@ -37,6 +37,7 @@ import org.labkey.experiment.api.ExpRunImpl; import javax.imageio.ImageIO; +import java.lang.ref.Cleaner; import java.awt.image.BufferedImage; import java.io.BufferedReader; import java.io.File; @@ -765,6 +766,40 @@ private static String getBaseFileName(ExpRun run, boolean detail, String focus) return fileName; } + private static final Cleaner CLEANER = Cleaner.create(); + + private static class FileLockState implements Runnable + { + private final Throwable _allocation; + private Lock _lock; + + private FileLockState(Throwable allocation, Lock lock) + { + _allocation = allocation; + _lock = lock; + } + + @Override + public void run() + { + if (_lock != null) + { + _log.error("Lock was not released. Creation was at:", _allocation); + _lock.unlock(); + _lock = null; + } + } + + private void release() + { + if (_lock != null) + { + _lock.unlock(); + _lock = null; + } + } + } + /** * Results for run graph generation. Must be released once the files have been consumed by the caller. */ @@ -772,14 +807,15 @@ public static class RunGraphFiles { private final File _mapFile; private final File _imageFile; - private Lock _lock; - private final Throwable _allocation = new Throwable(); + private final FileLockState _state; + private final Cleaner.Cleanable _cleanable; public RunGraphFiles(File mapFile, File imageFile, Lock lock) { _mapFile = mapFile; _imageFile = imageFile; - _lock = lock; + _state = new FileLockState(new Throwable(), lock); + _cleanable = CLEANER.register(this, _state); } public File getMapFile() @@ -792,27 +828,13 @@ public File getImageFile() return _imageFile; } - @Override - protected void finalize() throws Throwable - { - super.finalize(); - if (_lock != null) - { - _log.error("Lock was not released. Creation was at:", _allocation); - release(); - } - } - /** * Release the lock on the files. */ public void release() { - if (_lock != null) - { - _lock.unlock(); - _lock = null; - } + _state.release(); + _cleanable.clean(); } } } diff --git a/pipeline/src/org/labkey/pipeline/api/WorkDirectoryRemote.java b/pipeline/src/org/labkey/pipeline/api/WorkDirectoryRemote.java index 1029634155b..56c19ee62ef 100644 --- a/pipeline/src/org/labkey/pipeline/api/WorkDirectoryRemote.java +++ b/pipeline/src/org/labkey/pipeline/api/WorkDirectoryRemote.java @@ -16,6 +16,7 @@ package org.labkey.pipeline.api; import org.apache.logging.log4j.LogManager; +import java.lang.ref.Cleaner; import org.apache.logging.log4j.Logger; import org.labkey.api.pipeline.WorkDirFactory; import org.labkey.api.pipeline.WorkDirectory; @@ -544,6 +545,66 @@ public class FileLockCopyingResource extends SimpleCopyingResource private final Throwable _creationStack; private Lock _memoryLock; + private static final Cleaner CLEANER = Cleaner.create(); + + private static class LockState implements Runnable + { + private FileChannel _channel; + private FileLock _lock; + private Lock _memoryLock; + private final Throwable _creationStack; + + private LockState(FileChannel channel, FileLock lock, Lock memoryLock, Throwable creationStack) + { + _channel = channel; + _lock = lock; + _memoryLock = memoryLock; + _creationStack = creationStack; + } + + @Override + public void run() + { + if (_lock != null || _memoryLock != null) + { + _systemLog.error("FileLockCopyingResource was not released before it was garbage collected. Creation stack is: ", _creationStack); + close(); + } + } + + private void close() + { + try + { + if (_lock != null) + { + _lock.release(); + _lock = null; + } + if (_channel != null) + { + _channel.close(); + _channel = null; + } + } + catch (IOException e) + { + _systemLog.error("Failed to release lock", e); + } + finally + { + if (_memoryLock != null) + { + _memoryLock.unlock(); + _memoryLock = null; + } + } + } + } + + private final LockState _state; + private final Cleaner.Cleanable _cleanable; + public FileLockCopyingResource(FileChannel channel, int lockNumber, File f) throws IOException { _channel = channel; @@ -556,34 +617,23 @@ public FileLockCopyingResource(FileChannel channel, int lockNumber, File f) thro // Lock the file part second _lock = _channel.lock(); - } - @Override - protected void finalize() throws Throwable - { - super.finalize(); - if (_lock != null) - { - _systemLog.error("FileLockCopyingResource was not released before it was garbage collected. Creation stack is: ", _creationStack); - } - close(); + _state = new LockState(_channel, _lock, _memoryLock, _creationStack); + _cleanable = CLEANER.register(this, _state); } + @Override public void close() { if (_lock != null) { - // Unlock the file part first - try { _lock.release(); } catch (IOException e) {} - try { _channel.close(); } catch (IOException e) {} + _state.close(); + _cleanable.clean(); _jobLog.debug("Lock #" + _lockNumber + " released"); _lock = null; _channel = null; super.close(); - - // Unlock the memory part last - _memoryLock.unlock(); _memoryLock = null; } } diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index a25d7f875f7..0ac394da7ac 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -1755,26 +1755,26 @@ else if (ti instanceof LinkedTableInfo) { JdbcMetaDataSelector columnSelector = new JdbcMetaDataSelector(locator, (dbmd, l) -> dbmd.getColumns(l.getCatalogName(), l.getSchemaNamePattern(), l.getTableNamePattern(), null)); - result.addView(new ResultSetView(CachedResultSets.create(columnSelector.getResultSet(), true, Table.ALL_ROWS), "Table Meta Data")); + result.addView(new ResultSetView(CachedResultSets.create(columnSelector.getResultSet(), true, true, Table.ALL_ROWS), "Table Meta Data")); JdbcMetaDataSelector pkSelector = new JdbcMetaDataSelector(locator, (dbmd, l) -> dbmd.getPrimaryKeys(l.getCatalogName(), l.getSchemaName(), l.getTableName())); - result.addView(new ResultSetView(CachedResultSets.create(pkSelector.getResultSet(), true, Table.ALL_ROWS), "Primary Key Meta Data")); + result.addView(new ResultSetView(CachedResultSets.create(pkSelector.getResultSet(), true, true, Table.ALL_ROWS), "Primary Key Meta Data")); if (dialect.canCheckIndices(ti)) { JdbcMetaDataSelector indexSelector = new JdbcMetaDataSelector(locator, (dbmd, l) -> dbmd.getIndexInfo(l.getCatalogName(), l.getSchemaName(), l.getTableName(), false, false)); - result.addView(new ResultSetView(CachedResultSets.create(indexSelector.getResultSet(), true, Table.ALL_ROWS), "Other Index Meta Data")); + result.addView(new ResultSetView(CachedResultSets.create(indexSelector.getResultSet(), true, true, Table.ALL_ROWS), "Other Index Meta Data")); } JdbcMetaDataSelector ikSelector = new JdbcMetaDataSelector(locator, (dbmd, l) -> dbmd.getImportedKeys(l.getCatalogName(), l.getSchemaName(), l.getTableName())); - result.addView(new ResultSetView(CachedResultSets.create(ikSelector.getResultSet(), true, Table.ALL_ROWS), "Imported Keys Meta Data")); + result.addView(new ResultSetView(CachedResultSets.create(ikSelector.getResultSet(), true, true, Table.ALL_ROWS), "Imported Keys Meta Data")); JdbcMetaDataSelector ekSelector = new JdbcMetaDataSelector(locator, (dbmd, l) -> dbmd.getExportedKeys(l.getCatalogName(), l.getSchemaName(), l.getTableName())); - result.addView(new ResultSetView(CachedResultSets.create(ekSelector.getResultSet(), true, Table.ALL_ROWS), "Exported Keys Meta Data")); + result.addView(new ResultSetView(CachedResultSets.create(ekSelector.getResultSet(), true, true, Table.ALL_ROWS), "Exported Keys Meta Data")); } return result; } @@ -1827,7 +1827,7 @@ public ModelAndView getView(Object form, BindException errors) throws Exception ActionURL url = new ActionURL(RawTableMetaDataAction.class, getContainer()) .addParameter("schemaName", _schemaName) .addParameter("query.queryName", null); - tablesView = new ResultSetView(CachedResultSets.create(selector.getResultSet(), true, Table.ALL_ROWS), "Tables", "TABLE_NAME", url) + tablesView = new ResultSetView(CachedResultSets.create(selector.getResultSet(), true, true, Table.ALL_ROWS), "Tables", "TABLE_NAME", url) { @Override protected boolean shouldLink(ResultSet rs) throws SQLException diff --git a/query/src/org/labkey/query/jdbc/QueryStatement.java b/query/src/org/labkey/query/jdbc/QueryStatement.java index abb6f62a0ba..6e3db0aa751 100644 --- a/query/src/org/labkey/query/jdbc/QueryStatement.java +++ b/query/src/org/labkey/query/jdbc/QueryStatement.java @@ -45,13 +45,6 @@ public class QueryStatement implements Statement _conn = conn; } - @Override - protected void finalize() throws Throwable - { - assert null == _rs; - assert _closed; - super.finalize(); - } @Override public ResultSet executeQuery(String s) throws SQLException diff --git a/query/src/org/labkey/query/olap/OlapSchemaDescriptor.java b/query/src/org/labkey/query/olap/OlapSchemaDescriptor.java index 5e2f8db8311..fbe82c902e2 100644 --- a/query/src/org/labkey/query/olap/OlapSchemaDescriptor.java +++ b/query/src/org/labkey/query/olap/OlapSchemaDescriptor.java @@ -16,6 +16,7 @@ package org.labkey.query.olap; import org.apache.commons.io.IOUtils; +import java.lang.ref.Cleaner; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; @@ -48,6 +49,26 @@ */ public abstract class OlapSchemaDescriptor { + private static final Cleaner CLEANER = Cleaner.create(); + + private static class OlapState implements Runnable + { + private File _tmpFile; + + @Override + public void run() + { + if (null != _tmpFile) + { + _tmpFile.delete(); + _tmpFile = null; + } + } + } + + private final OlapState _state = new OlapState(); + private final Cleaner.Cleanable _cleanable = CLEANER.register(this, _state); + private final Module _module; private final String _id; private final String _name; @@ -256,6 +277,7 @@ File getFile() throws IOException if (null == tmpFile) { tmpFile = FileUtil.createTempFile("olap", getName()); + _state._tmpFile = tmpFile; tmpFile.deleteOnExit(); try ( FileOutputStream out = new FileOutputStream(tmpFile); @@ -268,18 +290,4 @@ File getFile() throws IOException return tmpFile; } - // TODO use ReferenceQueue to do cleanup instead of finalize() - @Override - protected void finalize() throws Throwable - { - try - { - if (null != tmpFile) - tmpFile.delete(); - } - finally - { - super.finalize(); - } - } }