jTDS icon indicating copy to clipboard operation
jTDS copied to clipboard

JtdsStatement finalizer waits for query to finish

Open TaylanUB opened this issue 3 years ago • 2 comments

I'm using jTDS in an Android application and if I interrupt a background thread running a query, it crashes my app with the following trace:

java.util.concurrent.TimeoutException: net.sourceforge.jtds.jdbc.JtdsStatement.finalize() timed out after 10 seconds
    at net.sourceforge.jtds.jdbc.JtdsConnection.releaseTds(JtdsConnection.java)
    at net.sourceforge.jtds.jdbc.JtdsStatement.close(JtdsStatement.java:972)
    at net.sourceforge.jtds.jdbc.JtdsStatement.finalize(JtdsStatement.java:219)
    at java.lang.Daemons$FinalizerDaemon.doFinalize(Daemons.java:222)
    at java.lang.Daemons$FinalizerDaemon.run(Daemons.java:209)
    at java.lang.Thread.run(Thread.java:761)

The releaseTds method is synchronized which makes me think this might be related to a deadlock. Or it might be waiting for some other long-running query to end.

TaylanUB avatar Sep 24 '21 10:09 TaylanUB

Upon further inspection, I think it's neither a deadlock nor waiting for another query. The finalizer of JtdsStatement calls close() on itself. The close() implementation calls reset() which calls closeAllResultSets() and the close() implementation of JtdsResultSet looks like this:

public void close() throws SQLException {
    if (!closed) {
        try {
            if (!getConnection().isClosed()) {
               // Skip to end of result set
               // Could send cancel but this is safer as
               // cancel could kill other statements in a batch.
               while (next());
            }
        } finally {
            closed = true;
            statement = null;
        }
    }
}

Long story short, closing a JtdsStatement means waiting for the query to finish and that even if the close() is called by the finalizer.

Maybe this is the least bad thing jTDS can do, or maybe the behavior could be improved, I'm not sure. I guess the finalizer shouldn't have to call close() in the first place, as it's the programmer's job to ensure resources are closed before they get garbage collected.

However, either way there's the question of how to actually abort a query. I've tried calling cancel() on the JtdsPreparedStatement but that doesn't seem to help. (I looked at its code but couldn't figure out what exactly it does.) I've checked what JtdsConnection.close() does, and it calls close() on all statements first, so that's not how to abort querying either.

So yeah, how do I actually abort a query with jTDS? (I'll change the title of this issue to reflect this related question.)

TaylanUB avatar Sep 24 '21 14:09 TaylanUB

Here's another update. Sorry about the mess. Comes out that JtdsStatement.cancel() works perfectly fine, and my code just wasn't calling it properly.

This means the only real issue is that the finalizer of JtdsStatement waits for queries to finish, which can take a very long time and consume excessive amounts of resources.

I'm leaving this issue open for now, but if there's truly nothing smarter that jTDS can do in that finalizer, feel free to close this.

I think that a call to cancel() might be appropriate in JtdsStatement.finalize(). And maybe it should also scream very loudly (print a big fat warning to System.err or something) when the finalizer notices that the Statement hasn't been cleanly cancelled and/or closed, so the programmer knows what's up.

TaylanUB avatar Sep 24 '21 17:09 TaylanUB