FreeTDS Development Group

[Alex Kiesel <alex.kiesel AT document-root.de>]

On Mon, 2004-01-19 at 12:29, Alex Kiesel wrote:
> Sqsh is not a multithreaded application, but it indeed issues a cancel
> within the signal handler. It does two thing from within this handler:
> * send a cancel
> * set a global variable which gets checked in all normal 'result
> processing loops'. Those loops are left when this variable is set to
> '1'. One excerpt:

Hi all,

I've written a patch, which should fix FreeTDS cancel behaviour in
ctlib.
The problem has indeed been in reading from / writing to the wire while
within an interrupt handler. When remodelling Sybase behaviour more
exact, that is, waiting for the next "normal access" to the data and
just then executing the cancellation, it seems to work fine.

After having a (short) look at dblib and odbc, I've come to the
conclusion that the problem is a pure ctlib problem and thus should be
fixed in ct.c, not in the tds layer:
* dblib does not have an asynchronous cancel (or am I wrong here)
* ODBC must handle multithreaded applications in SQLCancel which is
something completely different as what ctlib has to do.

Next to my patch, I'm sending a unittest for ctlib (t0010.c), which
simulates what I did manually with sqsh.

Could someone have a look at it, please?

Thank you,
-Alex
Index: src/ctlib/ct.c
===================================================================
RCS file: /home/cvs/mirror/freetds/freetds/src/ctlib/ct.c,v
retrieving revision 1.112
diff -u -r1.112 ct.c
--- src/ctlib/ct.c	26 Dec 2003 18:11:08 -0000	1.112
+++ src/ctlib/ct.c	26 Jan 2004 08:57:28 -0000
@@ -51,6 +51,7 @@
 static int _ct_get_client_type(int datatype, int usertype, int size);
 static int _ct_fetchable_results(CS_COMMAND * cmd);
 static int _ct_process_return_status(TDSSOCKET * tds);
+static int _ct_cancel_cleanup(CS_CONNECTION * conn);
 
 static int _ct_fill_param(CS_PARAM * param, CS_DATAFMT * datafmt, CS_VOID * data,
 			  CS_INT * datalen, CS_SMALLINT * indicator, CS_BYTE byvalue);
@@ -900,7 +901,14 @@
 	/* config flag is set.                                                */
 
 	for (;;) {
+		
+		if (cmd->con->pending_cancel == 1) {
+			_ct_cancel_cleanup(cmd->con);
 
+			/* Should we return success or failure? */
+			return CS_FAIL;
+		}
+		
 		tdsret = tds_process_result_tokens(tds, &res_type, &done_flags);
 
 		tdsdump_log(TDS_DBG_FUNC, "%L ct_results() process_result_tokens returned %d (type %d) \n",
@@ -1176,6 +1184,13 @@
 	TDS_INT temp_count;
 
 	tdsdump_log(TDS_DBG_FUNC, "%L ct_fetch()\n");
+	
+	if (cmd->con->pending_cancel == 1) {
+		_ct_cancel_cleanup(cmd->con);
+		
+		/* Should we return success or failure?? */
+		return CS_CMD_FAIL;
+	}
 
 	/* We'll call a special function for fetches from a cursor            */
 	/* the processing is too incompatible to patch into a single function */
@@ -1213,7 +1228,10 @@
 	if (cmd->curr_result_type == CS_CMD_FAIL)
 		return CS_CMD_FAIL;
 
-
+	if (cmd->con->tds_socket->state == TDS_IDLE) {
+		return CS_CMD_FAIL;
+	}
+	
 	marker = tds_peek(cmd->con->tds_socket);
 	if ((cmd->curr_result_type == CS_ROW_RESULT && marker != TDS_ROW_TOKEN) ||
 		(cmd->curr_result_type == CS_STATUS_RESULT && marker != TDS_RETURNSTATUS_TOKEN) )
@@ -1249,7 +1267,9 @@
 		}
 
 		/* have we reached the end of the rows ? */
-
+		if (cmd->con->tds_socket->state == TDS_IDLE)
+			return CS_FAIL;
+			
 		marker = tds_peek(cmd->con->tds_socket);
 
 		if (cmd->curr_result_type == CS_ROW_RESULT && marker != TDS_ROW_TOKEN)
@@ -1666,8 +1686,8 @@
 ct_cancel(CS_CONNECTION * conn, CS_COMMAND * cmd, CS_INT type)
 {
 	CS_RETCODE ret;
-
-	tdsdump_log(TDS_DBG_FUNC, "%L ct_cancel()\n");
+	
+	tdsdump_log(TDS_DBG_FUNC, "%L ct_cancel() called with type %d\n", type);
 	if (type == CS_CANCEL_CURRENT) {
 		if (conn || !cmd)
 			return CS_FAIL;
@@ -1691,7 +1711,22 @@
 	}
 	if (cmd)
 		conn = cmd->con;
-	if (conn && !IS_TDSDEAD(conn->tds_socket)) {
+	
+	if (type == CS_CANCEL_ATTN) {
+		conn->pending_cancel= 1;
+		return CS_SUCCEED;
+	}
+
+	return _ct_cancel_cleanup(conn);
+}
+
+CS_RETCODE
+_ct_cancel_cleanup(CS_CONNECTION * conn)
+{
+	tdsdump_log(TDS_DBG_FUNC, "%L _ct_cancel_cleanup\n");
+	conn->pending_cancel= 0;
+
+	if (!IS_TDSDEAD(conn->tds_socket)) {
 		tds_send_cancel(conn->tds_socket);
 		tds_process_cancel(conn->tds_socket);
 	}