Skip to content

Fix problems closing ResultSets #7328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-jeckels merged 2 commits into from
Jan 15, 2026
Merged

Fix problems closing ResultSets #7328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
731c639
Fix problems closing ResultSets in nested grid views
labkey-jeckels Jan 14, 2026
50cd1e0
Ensure superclass close() is called to do PHI logging
labkey-jeckels Jan 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 2 additions & 66 deletions api/src/org/labkey/api/data/GroupedResultSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,77 +37,13 @@
*/
public class GroupedResultSet extends ResultSetImpl
{
private int _columnIndex;
private final int _columnIndex;
private int _rowOffset = 0;
private int _lastRow = 0;
private int _groupCount = 0;

private boolean _ignoreNext;

public GroupedResultSet(ResultSet rs, String columnName, int maxRows, int maxGroups)
{
this(rs, columnName);
try
{
setMaxRows(maxRows);

if (maxRows > 0)
{
rs.last();
if (rs.getRow() > maxRows || (rs instanceof TableResultSet && !((TableResultSet)rs).isComplete()))
{
setComplete(false);
Object value = getObject(_columnIndex);
while (rs.previous() && value.equals(getObject(_columnIndex)))
{
// Don't need to do anything here
}
_lastRow = rs.getRow();
value = getObject(_columnIndex);
int groupCount = 1;
while (rs.previous())
{
if (!value.equals(getObject(_columnIndex)))
{
value = getObject(_columnIndex);
groupCount++;
}
}
_groupCount = groupCount;
}
rs.beforeFirst();
}

Object value = null;

if (maxRows > 0 && maxGroups > 0)
{
int groupingCount = 0;
while (rs.next())
{
if (!getObject(_columnIndex).equals(value))
{
value = getObject(_columnIndex);
groupingCount++;
if (groupingCount > maxGroups)
{
_groupCount = maxGroups;
_lastRow = getRow() - 1;
setComplete(false);
break;
}
}
}
rs.beforeFirst();
}
setComplete(_lastRow == 0);
}
catch (SQLException e)
{
throw new RuntimeSQLException(e);
}
}

public GroupedResultSet(ResultSet rs, String columnName, int maxGroups) throws SQLException
{
this(rs, columnName);
Expand Down Expand Up @@ -231,7 +167,7 @@ public NestedResultSet(ResultSet rs) throws SQLException
public void close()
{
// Rely on the outer result set for closing
_wasClosed = true;
close(false);
}

// Treat a change of value in the grouping column as the "end" of the ResultSet
Expand Down
71 changes: 33 additions & 38 deletions api/src/org/labkey/api/data/ResultSetImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,13 @@ public class ResultSetImpl extends LoggingResultSetWrapper implements TableResul
{
private static final Logger _log = LogManager.getLogger(ResultSetImpl.class);

private final @Nullable DbScope _scope;
private final @Nullable Connection _connection;
private final String _creatingThreadName;
private int _maxRows;
private boolean _countComplete;

private boolean _isComplete = true;

protected int _size;

// for resource tracking
private final StackTraceElement[] _debugCreated;
protected boolean _wasClosed = false;


private static final Cleaner CLEANER = Cleaner.create();

private static class ResultSetState implements Runnable
Expand Down Expand Up @@ -83,29 +75,35 @@ public void run()
if (!_wasClosed)
{
_log.error("ResultSet was not closed. Created by thread " + _creatingThreadName + " with stacktrace: " + ExceptionUtil.renderStackTrace(_debugCreated));
close();
close(true);
}
}

private void close()
private void close(boolean closeUnderlying)
{
if (!_wasClosed)
{
try
{
if (null != _scope)
if (closeUnderlying)
{
Statement stmt = _rs.getStatement();
_rs.close();
if (stmt != null)
if (null != _scope)
{
stmt.close();
Statement stmt = _rs.getStatement();
_rs.close();
if (stmt != null)
{
stmt.close();
}
if (_connection != null)
{
_scope.releaseConnection(_connection);
}
}
else
{
_rs.close();
}
_scope.releaseConnection(_connection);
}
else
{
_rs.close();
}
}
catch (SQLException e)
Expand Down Expand Up @@ -139,31 +137,22 @@ public ResultSetImpl(@Nullable Connection connection, @Nullable DbScope scope, R
super(rs, queryLogging);
MemTracker.getInstance().put(this);
// Capturing the full stack can be expensive so only do it when enabled
_debugCreated = MiniProfiler.getTroubleshootingStackTrace();
StackTraceElement[] debugCreated = MiniProfiler.getTroubleshootingStackTrace();
// Capturing the thread name is cheap and provides useful context for troubleshooting failures to close
_creatingThreadName = Thread.currentThread().getName();
String creatingThreadName = Thread.currentThread().getName();
_maxRows = maxRows;
try
{
if (connection != null)
if (connection == null && rs.getStatement() != null)
{
_connection = connection;
}
else if (rs.getStatement() != null)
{
_connection = rs.getStatement().getConnection();
}
else
{
_connection = null;
connection = rs.getStatement().getConnection();
}
}
catch (SQLException e)
{
throw new RuntimeSQLException(e);
}
_scope = scope;
_state = new ResultSetState(_creatingThreadName, _debugCreated, _scope, _connection, rs);
_state = new ResultSetState(creatingThreadName, debugCreated, scope, connection, rs);
_cleanable = CLEANER.register(this, _state);
}

Expand Down Expand Up @@ -198,6 +187,7 @@ protected boolean hasNext(boolean hasNext)
@Override
public int countAll() throws SQLException
{
//noinspection StatementWithEmptyBody
while(next());
return _size;
}
Expand Down Expand Up @@ -239,16 +229,21 @@ public boolean next() throws SQLException
@Override
public void close() throws SQLException
{
if (_wasClosed)
super.close();
close(true);
}

protected void close(boolean closeUnderlying)
{
if (_state._wasClosed)
{
if (ResultSetUtil.STRICT_CHECKING)
throw new IllegalStateException("ResultSet has already been closed!");
}
else
{
_state.close();
_state.close(closeUnderlying);
_cleanable.clean();
_wasClosed = true;
}
}

Expand All @@ -275,6 +270,6 @@ public Map<String, Object> getRowMap()
@Override
public @Nullable Connection getConnection()
{
return _connection;
return _state._connection;
}
}