Re: binary patch problems

On 17 September 2011 01:20, Mikko Tiihonen
<mikko.tiihonen@nitorcreations.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