curl-library
Re: [PATCH 2/7] pipelining: Fix connection handling under timeouts.
Date: Mon, 10 Nov 2014 11:25:14 +0100 (CET)
On Thu, 6 Nov 2014, Carlo Wood wrote:
> Data corruption (1420):
Lovely work on this problem Carlo! I have some questions on the specifics:
(First: I removed Curl_multi_set_easy_connection just now so this patch needs
a very minor fix. No biggie of course and I fixed my local copy.)
1 - Isn't the multi_do_connection_id check basically another way of checking
that the state hasn't reached DONE ? It seems weird to require a specific
connection to assume there's pending data to read, when you should be able to
use the state to check for that instead of this new field. Right?
2 - For a transfer that you abort due to a timeout, that is part of a pipeline
and there is data pending you set bits.timedout to prevent it from getting
more transfers added to the pipeline. But shouldn't you get the same effect by
setting bits.close and then avoid a new struct member for only this purpose?
3 - The changing of the state to DONE as Monty already commented on: I believe
there's a miniscule risk for a race condition here if the timeout is triggered
when already in the CURLM_STATE_DONE state, so I believe we need a check for
that and if already in DONE go to CURLM_STATE_COMPLETED instead. Your
thoughts?
-- / daniel.haxx.se ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.htmlReceived on 2014-11-10