Thread: patch: add a finalizer to AbstractJdbc1Statement
This patch adds a finalizer to AbstractJdbc1Statement that closes the statement. Without this, when server-side preparation is in use statements that are executed then discarded without an explicit close() will leak resources on the backend while that connection remains open, as a DEALLOCATE never gets executed. Objects with a finalizer are more expensive to create (depending on the VM). If it's too much of a price to pay in the general case, there's another approach that uses phantom references and only pays the cost when backend resources are actually allocated, but the code becomes much more complex. -O
Attachment
On 17/08/2003 15:00 Oliver Jowett wrote: > This patch adds a finalizer to AbstractJdbc1Statement that closes the > statement. Without this, when server-side preparation is in use > statements > that are executed then discarded without an explicit close() will leak > resources on the backend while that connection remains open, as a > DEALLOCATE > never gets executed. Not a good solution IMHO. Relying on GC to clean up resource leaks is a poor solution. And it you simply System.exit() the JVM, GC is not called at all. regards -- Paul Thomas +------------------------------+---------------------------------------------+ | Thomas Micro Systems Limited | Software Solutions for the Smaller Business | | Computer Consultants | http://www.thomas-micro-systems-ltd.co.uk | +------------------------------+---------------------------------------------+
On Mon, Aug 18, 2003 at 11:38:22AM +0100, Paul Thomas wrote: > > On 17/08/2003 15:00 Oliver Jowett wrote: > >This patch adds a finalizer to AbstractJdbc1Statement that closes the > >statement. Without this, when server-side preparation is in use > >statements > >that are executed then discarded without an explicit close() will leak > >resources on the backend while that connection remains open, as a > >DEALLOCATE > >never gets executed. > > Not a good solution IMHO. Relying on GC to clean up resource leaks is a > poor solution. I'm not suggesting that relying on GC for cleanup is a good idea. However, without this patch, the driver will *always* leak backend resources with longlived connections and leaky client code (which is possibly not under the control of the eventual owner of the connection -- e.g. the appserver case). > And it you simply System.exit() the JVM, GC is not called > at all. When the JVM exits, the physical connections go down so the backend will do resource cleanup on its own. -O
On 18/08/2003 12:11 Oliver Jowett wrote: > On Mon, Aug 18, 2003 at 11:38:22AM +0100, Paul Thomas wrote: > > > > On 17/08/2003 15:00 Oliver Jowett wrote: > > >This patch adds a finalizer to AbstractJdbc1Statement that closes the > > >statement. Without this, when server-side preparation is in use > > >statements > > >that are executed then discarded without an explicit close() will leak > > >resources on the backend while that connection remains open, as a > > >DEALLOCATE > > >never gets executed. > > > > Not a good solution IMHO. Relying on GC to clean up resource leaks is a > > poor solution. > > I'm not suggesting that relying on GC for cleanup is a good idea. > However, > without this patch, the driver will *always* leak backend resources with > longlived connections and leaky client code (which is possibly not under > the > control of the eventual owner of the connection -- e.g. the appserver > case). > > > And it you simply System.exit() the JVM, GC is not called > > at all. > > When the JVM exits, the physical connections go down so the backend will > do > resource cleanup on its own. This is the area where my knowledge of the back-end processing could be better (and I've not yet had sufficient time to "nail it" by grepping the back-end sources). Are server-side prepares tied to a connection? If yes, then I can easily see back-end resource clean-up happening. And this might provide us with another strategy for handling orphaned statements: I would envisage a scenario where the prepare-string/statement-name pair are cached in the connection. When a connection.prepareStatement(..) call is made, if there is already an entry in the cache, then use that otherwise prepare a new BE statement and add details to cache. This way, we would never need to worry about statement leaks as we wouldn't get duplicates in the BE connection. What we _would_ need to do is make PreparedStatement.close() a NOP when using server-side prepares as some connection pool implementations (I've been looking at Apache DBCP specifically as thats what I use ;-) call close on the underlying statement when close is called on their wrapper class (DelegatingPreparedStatement for DBCP). The actual DEALLOACTE would then be done on connection.close() (assuming we don't just let the BE take of it). Sorry if I've gone a bit OT regards -- Paul Thomas +------------------------------+---------------------------------------------+ | Thomas Micro Systems Limited | Software Solutions for the Smaller Business | | Computer Consultants | http://www.thomas-micro-systems-ltd.co.uk | +------------------------------+---------------------------------------------+
Patch applied.
thanks,
--Barry
Oliver Jowett wrote:
> This patch adds a finalizer to AbstractJdbc1Statement that closes the
> statement. Without this, when server-side preparation is in use statements
> that are executed then discarded without an explicit close() will leak
> resources on the backend while that connection remains open, as a DEALLOCATE
> never gets executed.
>
> Objects with a finalizer are more expensive to create (depending on the VM).
> If it's too much of a price to pay in the general case, there's another
> approach that uses phantom references and only pays the cost when backend
> resources are actually allocated, but the code becomes much more complex.
>
> -O
>
>
> ------------------------------------------------------------------------
>
> Index: src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.31
> diff -u -c -r1.31 AbstractJdbc1Statement.java
> *** src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java 11 Aug 2003 21:12:00 -0000 1.31
> --- src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java 17 Aug 2003 13:15:49 -0000
> ***************
> *** 721,726 ****
> --- 721,735 ----
> result = null;
> }
>
> + /**
> + * This finalizer ensures that statements that have allocated server-side
> + * resources free them when they become unreferenced.
> + */
> + protected void finalize() {
> + try { close(); }
> + catch (SQLException e) {}
> + }
> +
> /*
> * Filter the SQL string of Java SQL Escape clauses.
> *
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org