Partial solution for 'unexpected EOF' problem with pg_disconnect: it

seems we have a choice between annoying messages and leaking memory
(or dumping core, but that's right out).  Patch also fixes several
other problems in pg_disconnect, such as being willing to close a
channel that isn't a PG channel.

2 files changed, 36 insertions, 13 deletions

diff --git a/src/interfaces/libpgtcl/pgtclCmds.c b/src/interfaces/libpgtcl/pgtclCmds.c
index ca754688564..28680c90fc7 100644
--- a/src/interfaces/libpgtcl/pgtclCmds.c
+++ b/src/interfaces/libpgtcl/pgtclCmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/interfaces/libpgtcl/Attic/pgtclCmds.c,v 1.66 2002/09/02 21:51:47 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/interfaces/libpgtcl/Attic/pgtclCmds.c,v 1.67 2002/09/02 23:41:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -399,7 +399,6 @@ Pg_connect(ClientData cData, Tcl_Interp *interp, int argc, char *argv[])
 int
 Pg_disconnect(ClientData cData, Tcl_Interp *interp, int argc, char *argv[])
 {
-	Pg_ConnectionId *connid;
 	PGconn	   *conn;
 	Tcl_Channel conn_chan;
 
@@ -413,15 +412,14 @@ Pg_disconnect(ClientData cData, Tcl_Interp *interp, int argc, char *argv[])
 	if (conn_chan == NULL)
 	{
 		Tcl_ResetResult(interp);
-		Tcl_AppendResult(interp, argv[1], " is not a valid connection\n", 0);
+		Tcl_AppendResult(interp, argv[1], " is not a valid connection", 0);
 		return TCL_ERROR;
 	}
 
-#if TCL_MAJOR_VERSION >= 8
-	conn = PgGetConnectionId(interp, argv[1], &connid);
-	if (connid->notifier_channel != NULL)
-		Tcl_UnregisterChannel(interp, connid->notifier_channel);
-#endif
+	/* Check that it is a PG connection and not something else */
+	conn = PgGetConnectionId(interp, argv[1], (Pg_ConnectionId **) NULL);
+	if (conn == (PGconn *) NULL)
+		return TCL_ERROR;
 
 	return Tcl_UnregisterChannel(interp, conn_chan);
 }
diff --git a/src/interfaces/libpgtcl/pgtclId.c b/src/interfaces/libpgtcl/pgtclId.c
index f8b0d5c55bf..94bacfc0001 100644
--- a/src/interfaces/libpgtcl/pgtclId.c
+++ b/src/interfaces/libpgtcl/pgtclId.c
@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/interfaces/libpgtcl/Attic/pgtclId.c,v 1.33 2002/09/02 21:51:47 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/interfaces/libpgtcl/Attic/pgtclId.c,v 1.34 2002/09/02 23:41:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -144,6 +144,7 @@ static void
 PgWatchProc(ClientData instanceData, int mask)
 {
 }
+
 static int
 PgGetHandleProc(ClientData instanceData, int direction,
 				ClientData *handlePtr)
@@ -193,7 +194,7 @@ PgSetConnectionId(Tcl_Interp *interp, PGconn *conn)
 
 #if TCL_MAJOR_VERSION >= 8
 	connid->notifier_channel = Tcl_MakeTcpClientChannel((ClientData) PQsocket(conn));
-	Tcl_RegisterChannel(interp, connid->notifier_channel);
+	Tcl_RegisterChannel(NULL, connid->notifier_channel);
 #else
 	connid->notifier_socket = -1;
 #endif
@@ -227,6 +228,8 @@ PgGetConnectionId(Tcl_Interp *interp, char *id, Pg_ConnectionId ** connid_p)
 	{
 		Tcl_ResetResult(interp);
 		Tcl_AppendResult(interp, id, " is not a valid postgresql connection", 0);
+		if (connid_p)
+			*connid_p = NULL;
 		return (PGconn *) NULL;
 	}
 
@@ -286,6 +289,25 @@ PgDelConnectionId(DRIVER_DEL_PROTO)
 	connid->conn = NULL;
 
 	/*
+	 * Kill the notifier channel, too.  We must not do this until after
+	 * we've closed the libpq connection, because Tcl will try to close
+	 * the socket itself!
+	 *
+	 * XXX Unfortunately, while this works fine if we are closing due to
+	 * explicit pg_disconnect, Tcl versions through 8.3.3 dump core if we
+	 * try to do it during interpreter shutdown.  Not clear why, or if
+	 * there is a workaround.  For now, accept leakage of the (fairly
+	 * small) amount of memory taken for the channel state representation.
+	 * Note we are not leaking a socket, since libpq closed that already.
+	 */
+#ifdef NOT_USED
+#if TCL_MAJOR_VERSION >= 8
+	if (connid->notifier_channel != NULL)
+		Tcl_UnregisterChannel(NULL, connid->notifier_channel);
+#endif
+#endif
+
+	/*
 	 * We must use Tcl_EventuallyFree because we don't want the connid
 	 * struct to vanish instantly if Pg_Notify_EventProc is active for it.
 	 * (Otherwise, closing the connection from inside a pg_listen callback
@@ -782,8 +804,10 @@ PgStartNotifyEventSource(Pg_ConnectionId * connid)
 		if (pqsock >= 0)
 		{
 #if TCL_MAJOR_VERSION >= 8
-			Tcl_CreateChannelHandler(connid->notifier_channel, TCL_READABLE,
-							 Pg_Notify_FileHandler, (ClientData) connid);
+			Tcl_CreateChannelHandler(connid->notifier_channel,
+									 TCL_READABLE,
+									 Pg_Notify_FileHandler,
+									 (ClientData) connid);
 #else
 			/* In Tcl 7.5 and 7.6, we need to gin up a Tcl_File. */
 			Tcl_File	tclfile = Tcl_GetFile((ClientData) pqsock, TCL_UNIX_FD);
@@ -805,7 +829,8 @@ PgStopNotifyEventSource(Pg_ConnectionId * connid, bool allevents)
 	{
 #if TCL_MAJOR_VERSION >= 8
 		Tcl_DeleteChannelHandler(connid->notifier_channel,
-							 Pg_Notify_FileHandler, (ClientData) connid);
+								 Pg_Notify_FileHandler,
+								 (ClientData) connid);
 #else
 		/* In Tcl 7.5 and 7.6, we need to gin up a Tcl_File. */
 		Tcl_File	tclfile = Tcl_GetFile((ClientData) connid->notifier_socket,