Re: non-trivial finalize() on AbstractJdbc2Statement - Mailing list pgsql-jdbc

From Imran
Subject Re: non-trivial finalize() on AbstractJdbc2Statement
Date
Msg-id CAG6o=AH=NrCk7SZQ+zaKSehjajnoECn6YGs-YDW7S_SCv3FogQ@mail.gmail.com
Whole thread Raw
In response to Re: non-trivial finalize() on AbstractJdbc2Statement  ("Kevin Grittner" <Kevin.Grittner@wicourts.gov>)
Responses Re: non-trivial finalize() on AbstractJdbc2Statement  (Vitalii Tymchyshyn <tivv00@gmail.com>)
List pgsql-jdbc
Hi Guys

I dont think I'm referring to the speed or workings of the finalize() method available on AbstractJdbc2Statement. As said earlier I agree that it should return quickly if statements are closed by clients and also having safeguards around it looks nice. But my point is that by having a non-trivial finalize() an extra JdbcxStatement object is been created and referenced through a Finalizer, which is in a reference queue that would be picked up by the Finalizer thread. And a GC wouldn't reclaim the memory until the finalizer thread has done its work. As we have many queries executing, the main thread keeps filling this queue at a much faster rate than the finalizer thread (which is a low priority thread by default) would get to it. Under heavy load this paves the way for possible OOM issues as we've noticed. Also just to check this, we've patched up the driver we have without the finalize() method and we've seen much better GC and have not experienced OOM on similar load that resulted in OOMs. 

The general feeling as I've learned over the years is that finalize() should be used cautiously. So I was wondering if its really required for the AbstractJdbc2Statement to have a finalize() that only closes the statement, which one would expect the client to do anyway.

Cheers
-- Imran

On Mon, Feb 13, 2012 at 4:49 PM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote:
******* ********<tivv00@gmail.com> wrote:
> Looking at the code... Can it be because
> org.postgresql.jdbc2.AbstractJdbc2Statement#isClosed is not
> volatile? There is no synchronization and finalizer thread may
> simply not see statement was just closed by another thread.

That sounds likely enough to me.  I don't know whether declaring the
flag volatile would be enough, but it needs either that or access
only through synchronized blocks.

In addition, I would recommend something like the attached to make
the code more bullet-proof.

-Kevin


pgsql-jdbc by date:

Previous
From: Dave Cramer
Date:
Subject: Re: bug on to do list reproducable at version 9.0-801
Next
From: Vitalii Tymchyshyn
Date:
Subject: Re: non-trivial finalize() on AbstractJdbc2Statement