db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bryan Pendleton <bpendle...@amberpoint.com>
Subject Re: [jira] Updated: (DERBY-210) Network Server will leak prepared statements if not explicitly closed by the user until the connection is closed
Date Sun, 12 Feb 2006 17:29:28 GMT
Deepa Remesh (JIRA) wrote:
> I am attaching a draft patch 'derby-210-v2-draft.diff' for review. 

Hi Deepa,

I studied this patch for a while, focusing specifically on your bullet
points 8, 9, and 10. The changes look good to me, and I think you are
definitely on the right track.

I had just a few ideas/requests that I wanted to pass along:

- If you're still looking for ways to break the big patch into smaller
   ones, change #10 seems to me like it should be separable.
- Regarding change #10, it seems like "needsToSendParamData" is reset
   to false by rsClose(), which is called by close(). So maybe you don't
   need to reset it to false again in close(). In some ways, it
   seems like a simpler way to code change #10 would be to add

        currentDrdaRs = new DRDAResultSet();

   to DRDAStatement.rsClose(), rather than doing it in DRDAStatement.close().
   But I think the way you've done it is fine, too.
- In changes #8 and #9, it seems like it would be more consistent to name
   the new PreparedStatement method "markClosed" rather than "cleanupParameters".
   Since this markClosed() method would override the one in Statement, you'd
   need to be sure to call super.markClosed() during the method. It seems like,
   in addition to being more consistent with the superclass, this would also
   arrange for the new cleanup code to be called in a few more cases, such as
   when methods in the Connection class call Statement.markClosed().
- Lastly, it seems like one of the big parts of your change is to identify
   a few simple rules about which methods are safe to call from the finalizers,
   and which are not; namely, that finalizer-triggered code is purely local
   whereas close code may send messages to the server to call close code there.
   I think it would be good to add some comments to the Statement.java and
   PreparedStatement.java classes to record this analysis, so that future
   readers understand the distinction between the close() and markClosed()

This is great work you're doing here and I feel like we're in the final stretch
of a big improvement in the client behavior. Thanks much for all the effort!


View raw message