Skip site navigation (1) Skip section navigation (2)

Re: binary patch problems

From: Mikko Tiihonen <mikko(dot)tiihonen(at)nitorcreations(dot)com>
To: Dave Cramer <pg(at)fastcrypt(dot)com>
Cc: Oliver Jowett <oliver(at)opencloud(dot)com>, pgsql-jdbc(at)postgresql(dot)org
Subject: Re: binary patch problems
Date: 2011-09-20 14:00:41
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-jdbc
On 09/19/2011 01:03 PM, Dave Cramer wrote:
> Hi Mikko,
> My suggestion would be to fix it.

Finally had some time to look into this.
Closing the result set of the describe query fixes the test failure for me.


--- org/postgresql/jdbc2/	2011-09-20 16:55:18.130947602 +0300
+++ org/postgresql/jdbc2/	2011-09-20 16:56:02.310694214 +0300
@@ -541,6 +541,8 @@
          	int flags2 = flags | QueryExecutor.QUERY_DESCRIBE_ONLY;
          	StatementResultHandler handler2 = new StatementResultHandler();
          	connection.getQueryExecutor().execute(queryToExecute, queryParameters, handler2, 0, 0, flags2);
+                ResultWrapper result2 = handler2.getResults();
+                result2.getResultSet().close();

          StatementResultHandler handler = new StatementResultHandler();

> On Fri, Sep 16, 2011 at 7:14 PM, Oliver Jowett<oliver(at)opencloud(dot)com>  wrote:
>> On 17 September 2011 01:20, Mikko Tiihonen
>> <mikko(dot)tiihonen(at)nitorcreations(dot)com>  wrote:
>>> If I debugged it correctly it looks like the query statement is closed by
>>> finalizer during the test which in turn closes the relevant portals.
>>> If I add stmt.close() to the end of the test there are no failures as it
>>> keeps the GC from killing of the statement during the test.
>>> But why I only seem to get it when running with binary transfer patches I do
>>> not know. So there must be something in the patches that I cannot spot.
>> (I am halfway through my first coffee of the day. YMMV)
>> The test code holds a reference to the returned ResultSet, but doesn't
>> keep a reference to the Statement itself after calling executeQuery().
>> Normally this isn't a problem because there is a strong ref from a
>> ResultSet to its owning Statement, so while you are using a particular
>> ResultSet the creating statement remains reachable.
>> The problem seems to be introduced in the patch to executeQuery if
>> (ForceBinaryTransfers). This delegates to a new PreparedStatement
>> ("inner PreparedStatement") and splices its resultset ("inner
>> resultset") back into the surrounding Statement ("outer Statement")
>> and returns that.
>> However the inner ResultSet has the wrong parent Statement for that
>> context (ResultSet.getStatement() will return the "inner
>> PreparedStatement" not the "outer Statement") - which is a separate
>> problem, but also causes the GC problems as well, as now there's no
>> strong ref from the returned ResultSet to the Statement that the
>> client created it via.
>> So in the test code we end up with:
>>   (a) a strong ref to the inner ResultSet, directly; and
>>   (b) a strong ref to the inner PreparedStatement, via the statement
>> reference of the inner ResultSet.
>> .. but no strong ref to the outer Statement. So the outer Statement
>> becomes garbage right after the call to executeQuery(), and if you are
>> unlucky with GC timing, it can be finalized and closed under you,
>> which then closes the inner ResultSet.
>> Adding a call to stmt.close() means that the outer Statement still has
>> a strong reference directly from test code up until that point, so it
>> doesn't get finalized early.
>> At a glance, executeQuery() is also broken for queries that return
>> multiple resultsets if ForceBinaryTransfers is enabled - you only get
>> the first resultset back.
>> Oliver

In response to


pgsql-jdbc by date

Next:From: Oliver JowettDate: 2011-09-20 14:47:39
Subject: Re: binary patch problems
Previous:From: Radosław SmoguraDate: 2011-09-20 09:36:01
Subject: Re: behavior at the end of a transaction

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group