Re: non-trivial finalize() on AbstractJdbc2Statement

From: Imran <imranbohoran(at)gmail(dot)com>
To: Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov>
Cc: "******* ********" <tivv00(at)gmail(dot)com>, Dave Cramer <pg(at)fastcrypt(dot)com>, pgsql-jdbc(at)postgresql(dot)org
Subject: Re: non-trivial finalize() on AbstractJdbc2Statement
Date: 2012-02-15 11:11:17
Message-ID: CAG6o=AH=NrCk7SZQ+zaKSehjajnoECn6YGs-YDW7S_SCv3FogQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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(dot)Grittner(at)wicourts(dot)gov
> wrote:

> ******* ********<tivv00(at)gmail(dot)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
>
>

In response to

Responses

Browse pgsql-jdbc by date

  From Date Subject
Next Message Vitalii Tymchyshyn 2012-02-15 11:30:15 Re: non-trivial finalize() on AbstractJdbc2Statement
Previous Message Dave Cramer 2012-02-14 18:40:46 Re: bug on to do list reproducable at version 9.0-801