[#1011311] Patch for bug #1011310: Command timeouts after the first are not handled

View Trackers | Patches | Download .csv | Monitor

Date:
2013-02-21 14:48 Priority:
3

State:
Open

Submitted by:
Evan Martin (realityexists) Assigned to:
Nobody (None)

Category:
None Group:
None

Resolution:
Accepted

Summary:
Patch for bug #1011310: Command timeouts after the first are not handled

Detailed description
This patch removes NpgsqlConnector.CancelRequestCalled, added for bug #1010986, and replaces it with a parameter to the ProcessBackendResponsesEnum() method. This way there is no limit to the number of timeouts a single Connection instance can handle, but the original problem in #1010986 (Endless recursion on hanging connection in ProcessBackendResponsesEnum) should still be solved as well.

Followup

Message
Date: 2013-02-27 13:09 Sender: Francisco Figueiredo jr. Patch applied to cvs. Please, give it a try and let me know if it is ok. Thank you very much for your help and support.
Date: 2013-02-26 20:50 Sender: Francisco Figueiredo jr. I tested your patch and it worked great. Your test case was very helpful! Right now I'm at my job and I don't have access to cvs. As soon as I get back home I'll commit your patch. Thank you very much! Nice catch.
Date: 2013-02-26 11:31 Sender: Evan Martin Yes, I was trying to avoid adding any more state. So have you tested the patch? Is it OK?
Date: 2013-02-25 13:00 Sender: Francisco Figueiredo jr. That's true. I was thinking about a flag the NpgsqlCommand.Cancel sets, but your patch removes this flag. My bad :) I think another solution would reset this flag after processing the messages here: internal void ProcessBackendResponses(NpgsqlConnector context) { IterateThroughAllResponses(ProcessBackendResponsesEnum(context)); context.cancelrequestCalled = false; } But I think your solution is better because it removes one state to be maintained.
Date: 2013-02-25 12:32 Sender: Evan Martin No, I have not tested NpgsqlCommand.Cancel() with this patch, but my understanding of the code is that endless recursion could only occur with timeouts, not with the Cancel() method. Since the cancel request is not exposed to the user they cannot cancel that again.
Date: 2013-02-25 12:20 Sender: Francisco Figueiredo jr. Thanks for your patch! I'm checking it. It seems the problem is that the flag isn't properly reset. Did you check if your changes also work ok when the user directly cancels the query through the NpgsqlCommand.Cancel() method?

Attached Files:

Attachments:

Npgsql-1011310.patch

Changes:

Field	Old Value	Date	By
Resolution	None	2013-02-26 20:50	fxjr
File Added	794: Npgsql-1011310.patch	2013-02-21 14:48	realityexists