Skip to content

Conversation

@labkey-jeckels
Copy link
Contributor

@labkey-jeckels labkey-jeckels commented Jan 3, 2026

Rationale

We're upgrading to require Java 25.

Changes

  • Expect Java 25. Stop bothering to recognize Java 17.

@labkey-adam
Copy link
Contributor

@labkey-jeckels there are a couple other places that should be updated (RecompilingJspClassLoader, FileSystemWatcherImpl.TestCase). Find In Files @JavaRuntimeVersion to navigate to them.

@labkey-jeckels
Copy link
Contributor Author

@labkey-jeckels there are a couple other places that should be updated (RecompilingJspClassLoader, FileSystemWatcherImpl.TestCase). Find In Files @JavaRuntimeVersion to navigate to them.

Updated. I also eliminated finalize() methods.

Copy link
Contributor

@labkey-susanh labkey-susanh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm not fully understanding how the ConnectionState and Cleaner work since JavaDocs say the state should have no reference to the object being cleaned, but then I'm not sure how it can be used to clean anything.

In any case, I think the || true hacks want removing.

private final StackTraceElement[] _debugCreated;
private final @Nullable DbScope _scope;
private final @Nullable Connection _connection;
private final ResultSet _rs; // This is the underlying result set
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this OK? I thought the state was not supposed to have a reference to the object being cleaned?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're OK. The ResultSetImpl is the thing being cleaned. This member variable is the ResultSet that's being wrapped. See the use of rs in the call to the ResultSetState constructor vs this being passed to the cleaner on the next line.

These can be confusing and easy to mess up, so let me know what you think of my explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants