Calling Cancel on a running query causes execution of handlecancelrequest to fail

[smartguest]

When you click cancel from ADS, the task HandleCancelRequest is called, which then tries to call Cancel on the Query object. This then attempts to call Cancel on the cancellationtokensource. However after this, the rest of the code fails to execute and we do not get a response back from the task, causing ADS query run to hang. This needs to be investigated as to why this is not working.

[kburtram]

@smartguest I've assigned this issue to you since I assume this is part of what you're looking at as part of microsoft/azuredatastudio#3231. Please let me know if this isn't related to the Cancelation bug you're looking at in ADS.

[smartguest]

@kburtram This is related to it. Also it seems that the design of SqlClient currently does not allow for queries to be cancelled immediately if it goes into a infinite loop or long running query loops: caused by a deadlock in its code. This is if we use an asynchronous approach.

This is not the case with Sql Server Management Studio apparently, which uses a synchronous approach. This means we have to rewrite the code of SqlServerTools to use synchronous code, though this might cause issues.

Should I go ahead with rewriting the code of STS to use the SSMS approach or at least change the code to use synchronous code? The SqlClient used by STS does not currently allow for infinite or very long running queries to be cancelled at the moment for Async execution.

Here are the links discussing the issue, it has been over a couple of years since it was last addressed.

dotnet/SqlClient#44

https://stackoverflow.com/questions/48461567/canceling-query-with-while-loop-hangs-forever?noredirect=1#comment83955305_48461567

[kburtram]

@smartguest SSMS also uses SqlClient and is able to cancel queries correctly. I'm not sure what rewriting STS to use SSMS approach entails. Maybe better to discuss on a call. But at a high-level we should correct the STS to properly support cancelation.

Also, please note the category of ADS cancelation bugs are not primarily related to infinite loop scenarios. So if that one case is particularly complicated for some reason you may also want to understand why ADS can't properly cancel other types of queries as well.

[kburtram]

...I see some notes about synchronous code as being difference between SSMS/STS. SSMS uses many threads and I don't believe the query execution is occurring on the UI thread, though I'd have to double check to be sure. Perhaps let's look through what code you're referring to get better context on that comment.

[smartguest]

The code where execution happens is in Batch.cs in QueryExecution, STS uses an async task and readers to execute the command. According to the links I posted, the async functions do not work properly for infinite loop OR long running query scenarios from my understanding. Switching to synchronous execution might fix this issue. But yes, better to discuss on a call and to review the code first.

[kburtram]

Not sure the details on async methods and query cancelation. But assuming this is the case, then yes we should rewrite the async methods to be normal methods and wrap the query execution code path in a background task.

[smartguest]

In the dotnet bug report:

These two comments help explain the issue:

dotnet/SqlClient#44 (comment)

dotnet/SqlClient#44 (comment)

They also provided two workarounds, use sync APIs or timeout: dotnet/SqlClient#44 (comment) I am going to implement the sync APIs as the cancel button should immediately cancel after running, I might need help figuring out how to wrap the query execution code path, but for now I'll implement sync methods.

[kburtram]

@smartguest it sounds like removing the async methods is the route to go here. You may ping @saurabh500 if you have any questions since he appears to have context on this issue.