Sigh, I'm an idiot ... I broke the async startup logic a couple days ago,

tglsfdc · tglsfdc · commit d00391e7ac34 · 2000-01-16T21:18:52.000Z
by creating a race condition.  It wasn't waiting for select() to say
write-ready immediately after connect, which meant that you might get
an unhelpful 'broken pipe' error message if connect failed, rather than
the intended error message.
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v 1.110 2000/01/15 05:37:21 ishii Exp $
+ *	  $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v 1.111 2000/01/16 21:18:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -931,10 +931,16 @@ connectDBStart(PGconn *conn)
 static int
 connectDBComplete(PGconn *conn)
 {
-	PostgresPollingStatusType flag;
+	PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
+
+	if (conn == NULL || conn->status == CONNECTION_BAD)
+		return 0;
 
 	for (;;) {
-		flag = PQconnectPoll(conn);
+		/*
+		 * Wait, if necessary.  Note that the initial state (just after
+		 * PQconnectStart) is to wait for the socket to select for writing.
+		 */
 		switch (flag)
 		{
 			case PGRES_POLLING_ACTIVE:
@@ -964,6 +970,10 @@ connectDBComplete(PGconn *conn)
 				conn->status = CONNECTION_BAD;
 				return 0;
 		}
+		/*
+		 * Now try to advance the state machine.
+		 */
+		flag = PQconnectPoll(conn);
 	}
 }
 
@@ -1347,12 +1357,16 @@ PQconnectPoll(PGconn *conn)
  * Starts the process of passing the values of a standard set of environment
  * variables to the backend.
  *
- * ---------------- */
+ * ----------------
+ */
 PGsetenvHandle
 PQsetenvStart(PGconn *conn)
 {
 	struct pg_setenv_state *handle;
 
+	if (conn == NULL || conn->status == CONNECTION_BAD)
+		return NULL;
+
 	if ((handle = malloc(sizeof(struct pg_setenv_state))) == NULL)
 	{
 		printfPQExpBuffer(&conn->errorMessage,
@@ -1621,13 +1635,16 @@ int
 PQsetenv(PGconn *conn)
 {
 	PGsetenvHandle handle;
-	PostgresPollingStatusType flag;
+	PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
 
 	if ((handle = PQsetenvStart(conn)) == NULL)
 		return 0;
 
 	for (;;) {
-		flag = PQsetenvPoll(conn);
+		/*
+		 * Wait, if necessary.  Note that the initial state (just after
+		 * PQsetenvStart) is to wait for the socket to select for writing.
+		 */
 		switch (flag)
 		{
 			case PGRES_POLLING_ACTIVE:
@@ -1653,10 +1670,14 @@ PQsetenv(PGconn *conn)
 				break;
 
 			default:
-				/* Just in case we failed to set it in PQconnectPoll */
+				/* Just in case we failed to set it in PQsetenvPoll */
 				conn->status = CONNECTION_BAD;
 				return 0;
 		}
+		/*
+		 * Now try to advance the state machine.
+		 */
+		flag = PQsetenvPoll(conn);
 	}
 }
 

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@`
`7`	`7`	`*`
`8`	`8`	`*`
`9`	`9`	`* IDENTIFICATION`
`10`		`- * $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v 1.110 2000/01/15 05:37:21 ishii Exp $`
	`10`	`+ * $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v 1.111 2000/01/16 21:18:52 tgl Exp $`
`11`	`11`	`*`
`12`	`12`	`*-------------------------------------------------------------------------`
`13`	`13`	`*/`
`@@ -931,10 +931,16 @@ connectDBStart(PGconn *conn)`
`931`	`931`	`static int`
`932`	`932`	`connectDBComplete(PGconn *conn)`
`933`	`933`	`{`
`934`		`- PostgresPollingStatusType flag;`
	`934`	`+ PostgresPollingStatusType flag = PGRES_POLLING_WRITING;`
	`935`	`+`
	`936`	`+ if (conn == NULL \|\| conn->status == CONNECTION_BAD)`
	`937`	`+ return 0;`
`935`	`938`
`936`	`939`	`for (;;) {`
`937`		`- flag = PQconnectPoll(conn);`
	`940`	`+ /*`
	`941`	`+ * Wait, if necessary. Note that the initial state (just after`
	`942`	`+ * PQconnectStart) is to wait for the socket to select for writing.`
	`943`	`+ */`
`938`	`944`	`switch (flag)`
`939`	`945`	`{`
`940`	`946`	`case PGRES_POLLING_ACTIVE:`
`@@ -964,6 +970,10 @@ connectDBComplete(PGconn *conn)`
`964`	`970`	`conn->status = CONNECTION_BAD;`
`965`	`971`	`return 0;`
`966`	`972`	`}`
	`973`	`+ /*`
	`974`	`+ * Now try to advance the state machine.`
	`975`	`+ */`
	`976`	`+ flag = PQconnectPoll(conn);`
`967`	`977`	`}`
`968`	`978`	`}`
`969`	`979`
`@@ -1347,12 +1357,16 @@ PQconnectPoll(PGconn *conn)`
`1347`	`1357`	`* Starts the process of passing the values of a standard set of environment`
`1348`	`1358`	`* variables to the backend.`
`1349`	`1359`	`*`
`1350`		`- * ---------------- */`
	`1360`	`+ * ----------------`
	`1361`	`+ */`
`1351`	`1362`	`PGsetenvHandle`
`1352`	`1363`	`PQsetenvStart(PGconn *conn)`
`1353`	`1364`	`{`
`1354`	`1365`	`struct pg_setenv_state *handle;`
`1355`	`1366`
	`1367`	`+ if (conn == NULL \|\| conn->status == CONNECTION_BAD)`
	`1368`	`+ return NULL;`
	`1369`	`+`
`1356`	`1370`	`if ((handle = malloc(sizeof(struct pg_setenv_state))) == NULL)`
`1357`	`1371`	`{`
`1358`	`1372`	`printfPQExpBuffer(&conn->errorMessage,`
`@@ -1621,13 +1635,16 @@ int`
`1621`	`1635`	`PQsetenv(PGconn *conn)`
`1622`	`1636`	`{`
`1623`	`1637`	`PGsetenvHandle handle;`
`1624`		`- PostgresPollingStatusType flag;`
	`1638`	`+ PostgresPollingStatusType flag = PGRES_POLLING_WRITING;`
`1625`	`1639`
`1626`	`1640`	`if ((handle = PQsetenvStart(conn)) == NULL)`
`1627`	`1641`	`return 0;`
`1628`	`1642`
`1629`	`1643`	`for (;;) {`
`1630`		`- flag = PQsetenvPoll(conn);`
	`1644`	`+ /*`
	`1645`	`+ * Wait, if necessary. Note that the initial state (just after`
	`1646`	`+ * PQsetenvStart) is to wait for the socket to select for writing.`
	`1647`	`+ */`
`1631`	`1648`	`switch (flag)`
`1632`	`1649`	`{`
`1633`	`1650`	`case PGRES_POLLING_ACTIVE:`
`@@ -1653,10 +1670,14 @@ PQsetenv(PGconn *conn)`
`1653`	`1670`	`break;`
`1654`	`1671`
`1655`	`1672`	`default:`
`1656`		`- /* Just in case we failed to set it in PQconnectPoll */`
	`1673`	`+ /* Just in case we failed to set it in PQsetenvPoll */`
`1657`	`1674`	`conn->status = CONNECTION_BAD;`
`1658`	`1675`	`return 0;`
`1659`	`1676`	`}`
	`1677`	`+ /*`
	`1678`	`+ * Now try to advance the state machine.`
	`1679`	`+ */`
	`1680`	`+ flag = PQsetenvPoll(conn);`
`1660`	`1681`	`}`
`1661`	`1682`	`}`
`1662`	`1683`