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);