[Zope-dev] Solid DA patches to handle closed conns

Andrew M. Kuchling akuchlin@mems-exchange.org
Tue, 24 Aug 1999 15:05:50 -0400 (EDT)


Last month I reported that the Solid DA choked when the database
server closed the connection on it.  Today I looked into it and have
some patches (included below); they fix some fairly serious problems,
but more still remain.

It turns out that ZSolidDA/db.py actually had code to reopen closed
connections, but it was never triggered because an except statement
used the wrong exception object.  Fixing this slip exposed worse
problems than the original typo.

The first and most critical problem is that when you free a Statement
object whose database connection has been closed, you get a core dump.
This doesn't seem to be the DA's fault, but rather stems from the
libraries in Solid 2.3.  I couldn't fix the core dump, so instead the
patch rearranges ZSolidDA/db.py to only create self.stmt when it's
required, and doesn't copy self.stmt into a local variable, because
the statement object might change along the way if .connect() gets
called.

The second fix is to the C module; the _dealloc() routine for 
Connection objects raises an exception if there's an error on the
SQLDisconnect(), including if the connection was already closed.  The
patch to pysql.c suppresses the exception if it was a
connection-related error.

The patches seem to work in my tests, but my findings cast an
extremely disturbing light on the stability of Solid's libraries.

-- 
A.M. Kuchling			http://starship.python.net/crew/amk/
Advertising reaches out to touch the fantasy part of people's lives. And you
know, most people's fantasies are pretty sad.
    -- Frederik Pohl, _The Way The Future Was_

*** db.py	1999/08/24 16:03:40	1.1
--- db.py	1999/08/24 18:46:50
***************
*** 97,102 ****
--- 97,104 ----
  
  
+ import sql
  from sql import SQLConnection, SQLDataSources, SQLDescribeCol, SQLExecDirect
  from sql import SQLFetch, SQLFreeStmt, SQLGetString, SQLNumResultCols
+ from sql import SQLDisconnect
  from sql import SQLSetConnectOption, SQLTables, SQLTransact, SQL_AUTOCOMMIT
  from sql import SQL_AUTOCOMMIT_OFF, SQL_BIGINT, SQL_BINARY, SQL_BIT, SQL_CHAR
***************
*** 177,185 ****
          self._connect()
          
      def _connect(self):
          self.connection=c=apply(SQLConnection, self._connection_info)
  	try: SQLSetConnectOption(c,SQL_AUTOCOMMIT,SQL_AUTOCOMMIT_OFF)
  	except: pass # Oh well
-         self.stmt=c()
  
      def _apply(self, f, args):
--- 179,192 ----
          self._connect()
  
+ 
      def _connect(self):
+         if hasattr(self, 'connection'):
+             try:
+                 del self.stmt
+                 del self.connection
+             except: pass
          self.connection=c=apply(SQLConnection, self._connection_info)
  	try: SQLSetConnectOption(c,SQL_AUTOCOMMIT,SQL_AUTOCOMMIT_OFF)
  	except: pass # Oh well
  
      def _apply(self, f, args):
***************
*** 187,198 ****
          # Checks for stale connections.
          try: return apply(f, args)
! 	except error, v:
              try:
                  tb=sys.exc_traceback
                  state, native, mess = v
                  if state[:2]=='08':
!                     # Hm, spells like a connection failure.
                      # Let's connect and try again.
                      self._connect()
                      return apply(f, (self.stmt, ) + args[1:] )
                  raise error, v, tb
--- 194,208 ----
          # Checks for stale connections.
          try: return apply(f, args)
! 	except (error, sql.error), v:
              try:
                  tb=sys.exc_traceback
                  state, native, mess = v
                  if state[:2]=='08':
!                     # Hm, smells like a connection failure.
                      # Let's connect and try again.
                      self._connect()
+                     # _connect() creates a new value of self.stmt, so
+                     # we need to use args with the new value of self.stmt.
+                     self.stmt = self.connection()
                      return apply(f, (self.stmt, ) + args[1:] )
                  raise error, v, tb
***************
*** 200,203 ****
--- 210,214 ----
  
      def table_info(self):
+         self.stmt = self.connection()
          self._apply(SQLTables,(self.stmt, None, None, None, 'TABLE'))
  	r=[]
***************
*** 212,223 ****
  
      def tables(self, Qualifier=None, Owner=None, Name=None, Type=None, rdb=1):
  	self._apply(SQLTables,(self.stmt, Qualifier, Owner, Name, Type))
  	return self.query(rdb=rdb)
  
      def query(self, src=None, max_rows=99999999, rdb=1):
- 	stmt=self.stmt
  	self._register()
  	try:
  	    if src is not None:
  		r=filter(strip,split(src,'\0'))
  		if not r: raise ValueError, 'null sql'
--- 223,235 ----
  
      def tables(self, Qualifier=None, Owner=None, Name=None, Type=None, rdb=1):
+         self.stmt = self.connection()
  	self._apply(SQLTables,(self.stmt, Qualifier, Owner, Name, Type))
  	return self.query(rdb=rdb)
  
      def query(self, src=None, max_rows=99999999, rdb=1):
  	self._register()
  	try:
  	    if src is not None:
+                 self.stmt = self.connection()
  		r=filter(strip,split(src,'\0'))
  		if not r: raise ValueError, 'null sql'
***************
*** 225,230 ****
  		    res=None
  		    for s in r:
! 			self._apply(SQLExecDirect,(stmt,s))
! 			status, ncol = SQLNumResultCols(stmt)
  			if ncol:
  			    if res is not None:
--- 237,242 ----
  		    res=None
  		    for s in r:
! 			self._apply(SQLExecDirect,(self.stmt,s))
! 			status, ncol = SQLNumResultCols(self.stmt)
  			if ncol:
  			    if res is not None:
***************
*** 237,247 ****
  		    return res
  
! 		self._apply(SQLExecDirect,(stmt,src))
  
! 	except error, v:
  	    state, native, mess = v
  	    raise error, "%s (%s)" % (mess, state)
  
! 	status, ncol = SQLNumResultCols(stmt)
  	if ncol==0:
  	    if rdb: return "x\n8s\n"
--- 249,260 ----
  		    return res
  
!                 self._apply(SQLExecDirect,(self.stmt,src))
  
! 	except KeyError, v:
  	    state, native, mess = v
  	    raise error, "%s (%s)" % (mess, state)
  
! 
! 	status, ncol = SQLNumResultCols(self.stmt)
  	if ncol==0:
  	    if rdb: return "x\n8s\n"
***************
*** 254,258 ****
  	binary_flags=[0]*ncol
  	for i in range(ncol):
! 	    status, name, tp, prec, scale, nullable=SQLDescribeCol(stmt,i+1)
  	    names[i]=name
  	    if prec < 1: prec=8
--- 267,271 ----
  	binary_flags=[0]*ncol
  	for i in range(ncol):
! 	    status, name, tp, prec, scale, nullable=SQLDescribeCol(self.stmt,i+1)
  	    names[i]=name
  	    if prec < 1: prec=8
***************
*** 269,276 ****
  	    r.append(join(row,'\t'))
  
! 	status=SQLFetch(stmt)
  	while status==SQL_SUCCESS:
  	    for i in range(ncol):
! 		v=SQLGetString(stmt,i+1,binary_flags[i])
  		if rdbdefs[i]=='t':
  		    if find(v,'\\') >= 0: v=join(split(v,'\\'),'\\\\')
--- 282,289 ----
  	    r.append(join(row,'\t'))
  
! 	status=SQLFetch(self.stmt)
  	while status==SQL_SUCCESS:
  	    for i in range(ncol):
! 		v=SQLGetString(self.stmt,i+1,binary_flags[i])
  		if rdbdefs[i]=='t':
  		    if find(v,'\\') >= 0: v=join(split(v,'\\'),'\\\\')
***************
*** 283,305 ****
  		for i in indexes: rd[names[i]]=row[i]
  	    r.append(rd)
! 	    status=SQLFetch(stmt)
  
! 	SQLFreeStmt(stmt, SQL_CLOSE);
  	if rdb: r=join(r,'\n')+'\n'
  	return r
  
      def columns(self, table_name):
- 	stmt=self.stmt
  	try: self._apply(SQLExecDirect,
!                          (stmt, 'select * from %s where 1=2' % table_name))
  	except error, v:
  	    state, native, mess = v
  	    raise error, "%s (%s)" % (mess, state)
  
! 	status, ncol = SQLNumResultCols(stmt)
  	r=[]
  	standard_type=tpnames.has_key
  	for i in range(ncol):
! 	    status, name, tp, prec, scale, nullable=SQLDescribeCol(stmt,i+1)
  	    if standard_type(tp): tp=tpnames[tp]
  	    else: tp="Non-standard type %s" % tp
--- 296,317 ----
  		for i in indexes: rd[names[i]]=row[i]
  	    r.append(rd)
! 	    status=SQLFetch(self.stmt)
  
! 	SQLFreeStmt(self.stmt, SQL_CLOSE);
  	if rdb: r=join(r,'\n')+'\n'
  	return r
  
      def columns(self, table_name):
  	try: self._apply(SQLExecDirect,
!                          (self.stmt, 'select * from %s where 1=2' % table_name))
  	except error, v:
  	    state, native, mess = v
  	    raise error, "%s (%s)" % (mess, state)
  
! 	status, ncol = SQLNumResultCols(self.stmt)
  	r=[]
  	standard_type=tpnames.has_key
  	for i in range(ncol):
! 	    status, name, tp, prec, scale, nullable=SQLDescribeCol(self.stmt,i+1)
  	    if standard_type(tp): tp=tpnames[tp]
  	    else: tp="Non-standard type %s" % tp
***************
*** 308,312 ****
  		      'Nullable': nullable and 'with Null' or ''})
  
! 	SQLFreeStmt(stmt, SQL_CLOSE);
  	return r
  
--- 320,324 ----
  		      'Nullable': nullable and 'with Null' or ''})
  
! 	SQLFreeStmt(self.stmt, SQL_CLOSE);
  	return r
  

*** /tmp/ZSolidDA/src/pysql.c	Mon Jan 11 11:43:17 1999
--- pysql.c	Tue Aug 24 14:41:17 1999
***************
*** 119,124 ****
        UNLESS(SQLDisconnect(self->connection) == SQL_SUCCESS &&
  	     SQLFreeConnect(self->connection) == SQL_SUCCESS)
! 	PyErr_SetString(PyExc_SQLError,
! 			"error in SQL connection destructor");
      }
    PyMem_DEL(self);
--- 119,141 ----
        UNLESS(SQLDisconnect(self->connection) == SQL_SUCCESS &&
  	     SQLFreeConnect(self->connection) == SQL_SUCCESS)
! 	{
! 	  char state[6];
! 	  SDWORD native;
! 	  char mess[SQL_MAX_MESSAGE_LENGTH];
! 	  SWORD lmess;
! 
! 	  SQLError(SQLEnv, self->connection, SQL_NULL_HSTMT, 
! 		   state,&native,
! 		   mess,SQL_MAX_MESSAGE_LENGTH-1,&lmess);
! 
! 	  /* Only raise an exception if the state code doesn't begin 
! 	     with '08'; '08' means a connection error, and we don't
! 	     care about connection errors when we're *closing* a connection.
! 	  */
! 	  if (state[0]=='0' && state[1] == '8')
! 	    {
! 	      PyErr_Clear();
! 	    }
! 	}
      }
    PyMem_DEL(self);