leaked ODBC statement handles
tenger at iseries-guru.com
Wed Jul 18 06:55:21 PDT 2012
I have let myself write quite a bit in-line. Meanwhile, here are the
actual questions arising.
(*) Do you agree that ODatabaseMetaDataResultSet ctor should warn if
it does not get a statement handle?
(*) Is there any value to a bug report at this stage?
On Wed, 2012-07-18 at 05:15 +0200, Lionel Elie Mamane wrote:
> On Tue, Jul 17, 2012 at 03:46:14PM -0400, Terrence Enger wrote:
> > On Tue, 2012-07-17 at 19:08 +0200, Lionel Elie Mamane wrote:
> >> On Mon, Jul 16, 2012 at 10:15:02PM -0400, Terrence Enger wrote:
> >>> I am chasing some "leaked" ODBC statement handles.
> >>> I see that ODatabaseMetaDataResultSet.cxx takes care *not* to free
> >>> statement handle which has not been subjected to one of 13 member
> >>> functions with names starting "open...". Questions arising ...
> >> That is a bug. For the history, look at commit
> >> aa3f42832756b7165a0722b2d013a572acf224c8
> >> (...) I can easily believe the code was leaking statement handles
> >> in this way even back then (or
> > Well, I can demonstrate at least a leak. Removing m_bCloseHandle
> > of course fix the leak. Still, I wonder if the leak could be a sign
> > of a bug is client code somewhere. Thoughts?
> I'm not sure what you mean there.
Something created a ODatabaseMetaDataResultSet. Presumably the client
code had some intended purpose for the object. But the statement
handle appeared in the ODBC log file only in SQLAllocHandle. I
conclude that the client code did not do very much; as the client code
did not even free the handle, there is no sign that the client made a
deliberate decision to abandon its purpose. Hmm, that sounds like it
could be a bug, or an opportunity for optimization, or an opportunity
I, too, am not sure exactly what I mean: the backtraces from
construction of the otherwise-unused ODatabaseMetaDataResultSet
objects each show scores of call levels. Even assuming that my
hypothesized failed purpose is closer to the ctor than to main, I am
> >> double-freeing them or whatever).
> > Well, yes. I can imagine (but have not yet tried to demonstrate)
> > happening:
> > (*) Client code calls close, which calls dispose, which frees the
> > statement handle
> > (*) Client code calls dispose, which frees the statement handle a
> > second time. I have not looked at the Reference class since the
> > days of OO.o, but IIRC it calls dispose of the referenced class.
> > (*) The destructor calls dispose, which frees the statement handle a
> > third time..
> Good point. However, calling disposing() multiple times does not lead
> to double or triple freeing of the statement handle with the current
Oh my. I was wrong in so many ways ...
> ::osl::MutexGuard aGuard(m_aMutex);
> So, m_pConnection->freeStatementHandle can be called multiple times,
> OK. But look at OConnection::freeStatementHandle:
> void OConnection::freeStatementHandle(SQLHANDLE& _pHandle)
> _pHandle = SQL_NULL_HANDLE;
> 1) The argument is passed by reference
> 2) After freeing the handle, it sets the handle to a special dummy
> value; because it is passed by reference, it is updated in the
> caller, and DatabaseMetaDataResultSet::m_aStatementHandle is set to
> the dummy value.
> 3) So the second time disposing() is called, SQLFreeHandle is called
> on the dummy handle, not on the original "real" handle.
Exactly so. And moreover ...
(*) I confused ODatabaseMetaDataResultSet::dispose, which the dtor
calls, with ODatabaseMetaDataResultSet::disposing, which frees the
statement handle. Sheesh, it's not as if the names differed in
only one letter, or something.
(*) It was manifest in gdb that the dtor does not call disposing. I
was too close-minded to notice.
(*) The ODBC log files do not show any null handles.
Altogether, a bad day of work. Sorry.
> The remaining doubt I could have is whether calling free on the NULL
> handle is guaranteed safe; I don't see anything to that extent in the
> ODBC docs, maybe it would be safer to add:
> if (_pHandle == SQL_NULL_HANDLE)
> to the top of OConnection::freeStatementHandle?
Yes, I shall do that. It is more drastic what I was preparing to
offer in this function.
> > Removing m_bCloseHandle from ODatabaseMetaDataResultSet clears away
> > many questions that I was accumulating, and it makes somewhat
> > plausible my guess that the protocol for using
> > ODatabaseMetaDataResultSet is something like this:
> > construct
> > exactly one open<whatever>
> > in any order
> > | one or more get<whatever>
> > | zero or more calls repositioning the result set
> > | zero or more calls querying position of result set
> > exactly one close
> > exactly one dispose
> > destruct
> All these "exactly one" are too dangerous; as far as possible, the
> class should be safe for "zero or more" of anything :)
> Except for things like "exactly one construct() before anything else",
> which are pretty much guaranteed by C++ semantics :)
> > If the guess is plausible, should the class complain (SAL_WARN_IF, I
> > presume) about violations of the protocol?
> If there are legitimate protocol limitations, it is good, at the very
> least in debug mode, to check for protocol violations and react
> accordingly; from a warning to an exception, as appropriate.
> My first guess is that there are no such legitimate limitations here,
> but you've looked at this code more than me; if you think there are,
> let me know.
The removal of m_bCloseHandle makes it much easier to understand the
code, so I no longer feel the need to invent elaborate hypotheses.
Add your skepticism to this, and I think the question is dead.
(I may hack the class to warn of an object essentially unused
throughout its lifetime, but that is a bit of ad-hocery that should
not be inflicted on anybody else.)
I am making one more gratuitous change: ODatabaseMetaDataResultSet
ctor warns if it does not get a statment handle. The justification is
... well, I guess it's just my nast, suspicious mind. May I have your
Thank you for your patience with me.
More information about the LibreOffice