dev-list notifications (was: Re: [CHANGEUPLOAD FAILED] [ONGERRIT] leaked ODBC statement handles)
On 10.08.2012 17:50, Bjoern Michaelsen wrote: rationale: We are now sending a mail to the dev-list for every patch anyway. It does not seem to send a mail to the dev-list, at least with [CHERRYPICK]. I received: From: Gerrit ger...@gerrit.libreoffice.org CC: Ivan Timofeev timofeev@gmail.com [...] From LibreOffice gerrit bot ger...@libreoffice.org: LibreOffice gerrit bot has uploaded a new change for review. [...] i.e. no dev-list in CC. Best regards, Ivan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[CHANGEUPLOAD FAILED] [ONGERRIT] leaked ODBC statement handles
2012-08-10 13:09:19,202 INFO Handling incoming mail from Terrence Enger ten...@iseries-guru.com 2012-08-10 13:09:19,202 INFO subject: '[ONGERRIT] leaked ODBC statement handles' 2012-08-10 13:09:19,202 INFO executing command: None, branch: master, commit: leaked 2012-08-10 13:09:19,202 ERROREmailCommand instance has no attribute 'do_None' ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [CHANGEUPLOAD FAILED] [ONGERRIT] leaked ODBC statement handles
Hi Terrence, Ah, I removed that ONGERRIT thing, please just use PATCH. rationale: We are now sending a mail to the dev-list for every patch anyway. So I would suggest to _not_ CC the mailing list, as that would mean: - One mail for you sending the patch - One mail from the patchpickup saying its good - One mail from gerrit that there is a new patch The last mail should be enough for the list (and you will get the first two). Best, Bjoern On Fri, Aug 10, 2012 at 01:09:19PM +, ger...@libreoffice.org wrote: 2012-08-10 13:09:19,202 INFO Handling incoming mail from Terrence Enger ten...@iseries-guru.com 2012-08-10 13:09:19,202 INFO subject: '[ONGERRIT] leaked ODBC statement handles' 2012-08-10 13:09:19,202 INFO executing command: None, branch: master, commit: leaked 2012-08-10 13:09:19,202 ERROREmailCommand instance has no attribute 'do_None' ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [CHANGEUPLOAD FAILED] [ONGERRIT] leaked ODBC statement handles
-- View this message in context: http://nabble.documentfoundation.org/CHANGEUPLOAD-FAILED-ONGERRIT-leaked-ODBC-statement-handles-tp4000433p4000479.html Sent from the Dev mailing list archive at Nabble.com. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [ONGERRIT] leaked ODBC statement handles
On Wed, Jul 18, 2012 at 03:09:55PM -0400, Terrence Enger wrote: Patch attached. I think it as we discussed. Thanks. Applied pushed to master. I split your patch into three commits: - avoid freeing the NULL handle - ODBMetaDataRS ctor: abort if statement handle allocation failed - stop some leaked statement handles In particular, in the ODBMetaDataRS ctor: abort if statement handle allocation failed commit, I moved the abort (exception throwing) to the top of the function instead of the bottom. At the bottom, it would leak the allocation in m_pRowStatusArray, and a reference count in m_pConnection (because acquire has already been called). By contrast, the other initialisations done before, in the initialisation list are safe: no heap allocation :) -- Lionel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [ONGERRIT] leaked ODBC statement handles
On Thu, 2012-07-19 at 07:18 -0400, Terrence Enger wrote: Again, thank you for all the help you have given me along the way to this small code contribution. Hey ! this really helps - it's really great to have people tackling bugs - there are really plenty to go around :-) Thanks ! Michael. -- michael.me...@suse.com , Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[ONGERRIT] leaked ODBC statement handles
Patch attached. I think it as we discussed.
I do not have OpenId, so I am not registered with gerrit, so I think
that I shall not be able to enter your as reviewer. Sorry to be
awkward.
Thank you for your patience with me through all the to-ing and fro-ing
so far.
Terry.
From c7e9585283ef989dc50fc12b90333fe60944f726 Mon Sep 17 00:00:00 2001
From: Terrence Enger ten...@iseries-guru.com
Date: Wed, 18 Jul 2012 14:46:11 -0400
Subject: [PATCH] stop some leaked statement handles
Change-Id: I06764e0569ea615e66de6cd5946614c7c538e60e
---
.../source/drivers/odbcbase/OConnection.cxx|3 ++
.../odbcbase/ODatabaseMetaDataResultSet.cxx| 22 ---
.../source/inc/odbc/ODatabaseMetaDataResultSet.hxx |3 +-
3 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/connectivity/source/drivers/odbcbase/OConnection.cxx b/connectivity/source/drivers/odbcbase/OConnection.cxx
index c7ed89b..8f362d0 100644
--- a/connectivity/source/drivers/odbcbase/OConnection.cxx
+++ b/connectivity/source/drivers/odbcbase/OConnection.cxx
@@ -541,6 +541,9 @@ SQLHANDLE OConnection::createStatementHandle()
// -
void OConnection::freeStatementHandle(SQLHANDLE _pHandle)
{
+if( SQL_NULL_HANDLE == _pHandle )
+return;
+
::std::map SQLHANDLE,OConnection*::iterator aFind = m_aConnections.find(_pHandle);
N3SQLFreeStmt(_pHandle,SQL_RESET_PARAMS);
diff --git a/connectivity/source/drivers/odbcbase/ODatabaseMetaDataResultSet.cxx b/connectivity/source/drivers/odbcbase/ODatabaseMetaDataResultSet.cxx
index 953ccc5..7417b68 100644
--- a/connectivity/source/drivers/odbcbase/ODatabaseMetaDataResultSet.cxx
+++ b/connectivity/source/drivers/odbcbase/ODatabaseMetaDataResultSet.cxx
@@ -66,7 +66,6 @@ ODatabaseMetaDataResultSet::ODatabaseMetaDataResultSet(OConnection* _pConnection
,m_nCurrentFetchState(0)
,m_bWasNull(sal_True)
,m_bEOF(sal_False)
-,m_bFreeHandle(sal_False)
{
OSL_ENSURE(m_pConnection,ODatabaseMetaDataResultSet::ODatabaseMetaDataResultSet: No parent set!);
osl_incrementInterlockedCount( m_refCount );
@@ -74,6 +73,9 @@ ODatabaseMetaDataResultSet::ODatabaseMetaDataResultSet(OConnection* _pConnection
m_pRowStatusArray = new SQLUSMALLINT[1]; // the default value
osl_decrementInterlockedCount( m_refCount );
// allocBuffer();
+
+if( SQL_NULL_HANDLE == m_aStatementHandle )
+throw RuntimeException();
}
// -
@@ -93,8 +95,8 @@ void ODatabaseMetaDataResultSet::disposing(void)
OPropertySetHelper::disposing();
::osl::MutexGuard aGuard(m_aMutex);
-if(m_bFreeHandle)
-m_pConnection-freeStatementHandle(m_aStatementHandle);
+
+m_pConnection-freeStatementHandle(m_aStatementHandle);
m_aStatement= NULL;
m_xMetaData.clear();
@@ -843,7 +845,6 @@ void ODatabaseMetaDataResultSet::openTables(const Any catalog, const ::rtl::OUS
const ::rtl::OUString tableNamePattern,
const Sequence ::rtl::OUString types ) throw(SQLException, RuntimeException)
{
-m_bFreeHandle = sal_True;
::rtl::OString aPKQ,aPKO,aPKN,aCOL;
const ::rtl::OUString *pSchemaPat = NULL;
@@ -891,7 +892,6 @@ void ODatabaseMetaDataResultSet::openTables(const Any catalog, const ::rtl::OUS
//-
void ODatabaseMetaDataResultSet::openTablesTypes( ) throw(SQLException, RuntimeException)
{
-m_bFreeHandle = sal_True;
SQLRETURN nRetcode = N3SQLTables(m_aStatementHandle,
0,0,
0,0,
@@ -908,7 +908,6 @@ void ODatabaseMetaDataResultSet::openTablesTypes( ) throw(SQLException, RuntimeE
// -
void ODatabaseMetaDataResultSet::openCatalogs() throw(SQLException, RuntimeException)
{
-m_bFreeHandle = sal_True;
SQLRETURN nRetcode = N3SQLTables(m_aStatementHandle,
(SDB_ODBC_CHAR *) SQL_ALL_CATALOGS,SQL_NTS,
(SDB_ODBC_CHAR *) ,SQL_NTS,
@@ -926,7 +925,6 @@ void ODatabaseMetaDataResultSet::openCatalogs() throw(SQLException, RuntimeExcep
// -
void ODatabaseMetaDataResultSet::openSchemas() throw(SQLException, RuntimeException)
{
-m_bFreeHandle = sal_True;
SQLRETURN nRetcode = N3SQLTables(m_aStatementHandle,
(SDB_ODBC_CHAR *) ,SQL_NTS,
(SDB_ODBC_CHAR *) SQL_ALL_SCHEMAS,SQL_NTS,
@@ -952,7 +950,6 @@ void ODatabaseMetaDataResultSet::openColumnPrivileges( const Any catalog, cons
else
pSchemaPat = NULL;
-m_bFreeHandle = sal_True;
::rtl::OString aPKQ,aPKO,aPKN,aCOL;
if ( catalog.hasValue() )
@@ -988,7 +985,6 @@
Re: leaked ODBC statement handles
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?
Thanks,
Terry.
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
a
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
http://cgit.freedesktop.org/libreoffice/core/commit/?id=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
will
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
for simplification.
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
daunted.
double-freeing them or whatever).
Well, yes. I can imagine (but have not yet tried to demonstrate)
this
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
code.
Oh my. I was wrong in so many ways ...
ODatabaseMetaDataResultSet::disposing:
::osl::MutexGuard aGuard(m_aMutex);
if(m_bFreeHandle)
m_pConnection-freeStatementHandle(m_aStatementHandle);
So, m_pConnection-freeStatementHandle can be called multiple times,
OK. But look at OConnection::freeStatementHandle:
void OConnection::freeStatementHandle(SQLHANDLE _pHandle)
{
N3SQLFreeStmt(_pHandle,SQL_RESET_PARAMS);
N3SQLFreeStmt(_pHandle,SQL_UNBIND);
N3SQLFreeStmt(_pHandle,SQL_CLOSE);
N3SQLFreeHandle(SQL_HANDLE_STMT,_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)
return;
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 openwhatever
in any order
| one or more getwhatever
| zero
Re: leaked ODBC statement handles
On Wed, Jul 18, 2012 at 09:55:21AM -0400, Terrence Enger wrote: (*) Do you agree that ODatabaseMetaDataResultSet ctor should warn if it does not get a statement handle? It never gets a statement handle as an argument, it allocates it by calling _pConnection-createStatementHandle(). If the latter fails, throw an exception, the ODatabaseMetaDataResultSet will be unusable anyway. 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 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 will 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 for simplification. I understand client code as code that creates a ODatabaseMetaDataResultSet. That code cannot possibly free the handle, because it cannot possibly know the handle, since it is a private value of ODatabaseMetaDataResultSet. However, indeed it creates a ODatabaseMetaDataResultSet and then does nothing with it, so it could be an opportunity for simplification / optimisation, yes. I've looked at the first 5 or so ODatabaseMetaDataResultSet ctor calls in ODatabaseMetaData.cxx, they look like when they don't do anything with the created ResultSet, it is an error fall-over, to just have an empty ResultSet to give their caller, which seems reasonable. But if you find a scenario where there is a more genuine object construction not needed here, that would be an interesting place to look at, yes. (*) 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. ::dispose calls ::disposing; see cppu::OComponentHelper::dispose in cppuhelper/source/component.cxx -- Lionel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: leaked ODBC statement handles
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 a 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 http://cgit.freedesktop.org/libreoffice/core/commit/?id=aa3f42832756b7165a0722b2d013a572acf224c8 Before that commit the statement handle was passed to the constructor; the class had not allocated the handle, and thus it was not the class's role to free it (I guess) unless it had taken ownership of it by actually using it. I'm muddy on how the calling code was supposed to know it should free it because ODatabaseMetaDataResultSet did not use it or it was supposed *not* to free it because ODBMetaDataRS used it; I can easily believe the code was leaking statement handles in this way even back then (or double-freeing them or whatever). Anyway, since this commit, the handle is allocated privately fresh by the class, and it should thus unconditionally free it on dispose; m_bFreeHandle should die. To you the honours of writing the patch? Put me as a reviewer, I'll apply it. -- Lionel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: leaked ODBC statement handles
I am sorry for the length. I hope some of it is interesting. 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 a 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 http://cgit.freedesktop.org/libreoffice/core/commit/?id=aa3f42832756b7165a0722b2d013a572acf224c8 Thank you for the link. Having that code brought together--no need to flip between windows or applications or to flip over intervening printed pages--quite changes the way I see it. Before that commit the statement handle was passed to the constructor; the class had not allocated the handle, and thus it was not the class's role to free it (I guess) unless it had taken ownership of it by actually using it. I'm muddy on how the calling code was supposed to know it should free it because ODatabaseMetaDataResultSet did not use it or it was supposed *not* to Yeah, I was looking at this yesterday, and the wee hours of this morning, and since I got up today. Muddy is the right word for the way I was feeling before I read your answer. free it because ODBMetaDataRS used it; 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 will of course fix the leak. Still, I wonder if the leak could be a sign of a bug is client code somewhere. Thoughts? double-freeing them or whatever). Well, yes. I can imagine (but have not yet tried to demonstrate) this 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.. Anyway, since this commit, the handle is allocated privately fresh by the class, and it should thus unconditionally free it on dispose; m_bFreeHandle should die. To you the honours of writing the patch? Put me as a reviewer, I'll apply it. Oh, goody. It is months since I submitted a patch to Base. I think, moreover, that I should not wait until I understand more deeply what is going on. I had already decided that m_bCloseHandle in the class definition needs a comment this is not what the name suggests, but I had real trouble deciding what the variable *does* mean. 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 openwhatever in any order | one or more getwhatever | zero or more calls repositioning the result set | zero or more calls querying position of result set exactly one close exactly one dispose destruct You caught me in the middle of thinking about question and writing code in ODatabaseMetaDataResultSet.cxx to shout out if the guess is false. Do you have a feeling for whether this guess is plausible? If the guess is plausible, should the class complain (SAL_WARN_IF, I presume) about violations of the protocol? If so, some questions come immediately to mind ... (*) Do we have an example of a class which checks explicitly the sequence of methods that the client calls? I do not want to introduce a second way of doing the same thing. (To introduce the 3rd or the seventeenth way is not such a big deal grin /.) If I need to invent a mechanism, I shall come back with questions. (*) Do we have an example of doxygen comments about such checking? Meanwhile, I am gonna to off into gdb and try to see repeated attempts to free a statement handle. Thanks, Terry. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: leaked ODBC statement handles
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 a
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
http://cgit.freedesktop.org/libreoffice/core/commit/?id=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 will
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.
double-freeing them or whatever).
Well, yes. I can imagine (but have not yet tried to demonstrate) this
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
code.
ODatabaseMetaDataResultSet::disposing:
::osl::MutexGuard aGuard(m_aMutex);
if(m_bFreeHandle)
m_pConnection-freeStatementHandle(m_aStatementHandle);
So, m_pConnection-freeStatementHandle can be called multiple times,
OK. But look at OConnection::freeStatementHandle:
void OConnection::freeStatementHandle(SQLHANDLE _pHandle)
{
N3SQLFreeStmt(_pHandle,SQL_RESET_PARAMS);
N3SQLFreeStmt(_pHandle,SQL_UNBIND);
N3SQLFreeStmt(_pHandle,SQL_CLOSE);
N3SQLFreeHandle(SQL_HANDLE_STMT,_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.
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)
return;
to the top of OConnection::freeStatementHandle?
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 openwhatever
in any order
| one or more getwhatever
| 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.
--
Lionel
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
leaked ODBC statement handles
Greetings, I am chasing some leaked ODBC statement handles. I see that ODatabaseMetaDataResultSet.cxx takes care *not* to free a statement handle which has not been subjected to one of 13 member functions with names starting open Questions arising ... (1) Why should this protection be necessary, does anyone know offhand? It is conceivable that some called function takes responsibility for freeing the statement handle. But I have looked through the present class for functions which reference the statement handle without setting the flag to free the handle, and I see only function names (I have not looked farther than the names) that make such transfer of responsibility sound unlikely: 10 functions with names like getsometype, 6 positioning functions (first, last, absolute, relative, previous, and next), and cancel (which, IIRC wraps SQLCancel). (2) Does anyone know offhand of a good reason for a client of the present class to instantiate it without apparently doing anything with the statement? The particular leaked statement handles that I see appear in the ODBC log file only as return values from SQLAllocHandle. (It is just possible that the failure to do anything with the handle is related to the funny query results that I was looking at when I noticed the leaked handles.) (3) Would it be good to report statement handles which disposing, as currently written, does not free? Is there a better place to do this than disposing? I imagine showing a few words and the handle itself. Is there something else which would be useful and easy to do? Thank you, all, for your attention. Terry. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
