FreeTDS Development Group

["James K. Lowden" <jklowden AT freetds.org>]

Stephane Magne wrote:
> Comparing the two, I noticed that the highlighted lines in the code  
> below appeared in the CS_CANCEL_ATTN block, but didn't appear in the  
> equivalent locations in the CS_CANCEL_ALL block.  So I just added the  
> two lines to the proper locations in the CS_CANCEL_ALL block of code  
> and it cancelled the query properly.

Really, "diff -u ct.c ct.c-orig" would have been clearer.  :-)  Mine's
attached.  Is that what you meant?  
  
I was confused because the CS_CANCEL_ALL branches all end with 

        tds_process_cancel(cmd_conn->tds_socket);
        _ct_initialise_cmd(cmd);

One might think that processing a cancel and initializing a command would
not be enhanced by tacking on 

        cmd->cancel_state = _CS_CANCEL_PENDING;

leaving the connection in "cancel pending" state.  But in fact the tds
layer --  i.e. tds_process_cancel only handles the cancel packet.  It
doesn't set the command state, which it shouldn't, because libtds doesn't
know which library called it or what state the ct-lib command should be
in.  There's a cleanup function it has to call before it can send another
query, and that state serves to notify the next-called function to do that
cleanup.  

So yours looks like a good fix.  Thanks!  

Unfortunately, src/unittests/cancel.c doesn't try CS_CANCEL_ALL.  I'd like
to see the test improved to verify the bug and the fix.   Any chance I
could persuade you to extend the unit test?  If not, thanks anyway for
pointing out the problem.  I updated TODO.  

--jkl

Attachment: ct_cancel.diff
Description: Binary data