Re: non-trivial finalize() on AbstractJdbc2Statement

From: Віталій Тимчишин <tivv00(at)gmail(dot)com>
To: Imran <imranbohoran(at)gmail(dot)com>
Cc: Dave Cramer <pg(at)fastcrypt(dot)com>, pgsql-jdbc(at)postgresql(dot)org
Subject: Re: non-trivial finalize() on AbstractJdbc2Statement
Date: 2012-02-13 15:23:10
Message-ID: CABWW-d3tz=Nhk3PtfkaQ1MEcU9XTDFDmLzmV3xgdQqfijcEk+g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-jdbc

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.

Best regards, Vitalii Tymchyshyn

2012/2/13 Imran <imranbohoran(at)gmail(dot)com>

> Hi Vitalli
>
> Yes my clients are definitely closing the PreparedStatements opened. In
> fact we are using the hibernate query interface and I've been through the
> code to make sure the close is been called in a finally block.
> I agree that closing work done by the finalizer() method would actually
> close the resource if its not closed. However the issue I'm raising is that
> by having the non-trivial finalize() method, it adds every
> PreparedStatement object created into the finalizer queue. And hence the a
> GC not reclaiming all memory is should. So before the finalizer thread
> (which is a low priority thread) gets to it, the application has ran out of
> memory.
>
> Cheers
> -- Imran
>
>
> On Mon, Feb 13, 2012 at 1:28 PM, Vitalii Tymchyshyn <tivv00(at)gmail(dot)com>wrote:
>
>> **
>> Hello.
>>
>> While it would be better to handle resource closing with reference
>> queues, are you sure your client does not leave resources unclosed? As for
>> me, driver in finalize should do only work not done because of close not
>> have been called correctly. In other words, your situation may be an
>> indication of close not been called, and so driver does required work in
>> finalize, so saturating finalize thread.
>>
>> Best regards, Vitalii Tymchyshyn
>>
>> 13.02.12 15:02, Imran написав(ла):
>>
>> Hi Dave
>>
>> Thanks for your response. I agree that this might not be widespread.
>> And I haven't seen much mentions around this problem (hitting GC issues due
>> to non-trivial finalize() methods) reported (based on my google searches).
>> However, I guess the existence of it does provide the opportunity for this
>> to happen as we've noticed. IMHO, encouraging clients to use the api
>> as recommended (i.e. closing resources once they are done with them) is
>> better than the driver allowing opportunity for bad things to happen. Or
>> perhaps the driver caters to this kind of situation using WeakReference as
>> oppose to using a non-trivial finalize().
>>
>> What are your thoughts?
>>
>> Cheers
>> -- Imran
>>
>> On Mon, Feb 13, 2012 at 12:09 PM, Dave Cramer <pg(at)fastcrypt(dot)com> wrote:
>>
>>> Well this is a typical which is worse case scenario. People leaking
>>> resources because they don't explicitly close them, or your case ? I'm
>>> not sure which is worse.
>>>
>>> Given that the driver is being used in many very high throughput sites
>>> without this problem, I'm curious as to why nobody else has complained
>>>
>>> Dave Cramer
>>>
>>> dave.cramer(at)credativ(dot)ca
>>> http://www.credativ.ca
>>>
>>>
>>>
>>> On Fri, Feb 10, 2012 at 12:39 PM, Imran <imranbohoran(at)gmail(dot)com> wrote:
>>> > Hello
>>> >
>>> > We've been having OOM errors in our applications through GC overhead
>>> limits
>>> > under heave load resources running queries. Inspecting the heap dump,
>>> it
>>> > appears that the finalizer queue is taken up most of the heap space.
>>> Almost
>>> > all of the the finalizer objects I've seen seem to have a
>>> > jdbc3PreparedStatement object in it. Going through the source code of
>>> the
>>> > driver I see that the 'AbstractJdbc2Statement' has a non-trivial
>>> finalize
>>> > method. I guess this explains why these objects end up in the finalizer
>>> > queue. Can I clarify the need to having this finalize() method here? It
>>> > seems to be calling the close() method of the statement which I would
>>> have
>>> > thought is the responsibility of the client building a Statement
>>> object. Is
>>> > there any chance this can be dropped so we don't see these objects
>>> ending up
>>> > in the finalizer queue under heavy load and the jvm running out of
>>> memory
>>> > before the GC threads gets around to 'actually' reclaim the memory?
>>> >
>>> > Also we are using postgres 9.0.4 and the 8.3-604.jdbc3 version of the
>>> > postgresql jdbc driver.
>>> >
>>> > Cheers
>>> > -- Imran
>>>
>>
>>
>>
>

--
Best regards,
Vitalii Tymchyshyn

In response to

Responses

Browse pgsql-jdbc by date

  From Date Subject
Next Message Kevin Grittner 2012-02-13 16:49:41 Re: non-trivial finalize() on AbstractJdbc2Statement
Previous Message Віталій Тимчишин 2012-02-13 15:19:46 Re: non-trivial finalize() on AbstractJdbc2Statement