Re: [PATCH] patch series for improved multi interface
Date: Wed, 18 Aug 2010 23:54:13 +0200 (CEST)
On Mon, 16 Aug 2010, Constantine Sapuntzakis wrote:
>> * We make Curl_expire() return a pointer to the newly added timeout struct.
>> * We introduce a Curl_rmexpire() (perhaps a better name is needed) that
>> more explicitly removes the given timer, or if set to NULL, the entire
>> list of timers. (I think the most common use of 0 is in fact to quite
>> accurately remove all timers.)
> This proposal is good. The original proposal might be simpler to code.
Only marginally. I think my proposal is an API that is easier to read.
> In this proposal, when would the newly allocated struct be freed?
Exactly like today, when all timeouts are cleared.
> In both proposals, you end up adding fields to the structs (e.g.
> SessionHandle or hostthre.c:thread_data). In the first proposal, it's an
> in-line timeout struct. In the proposal above, it's a pointer.
Yes, but it would be a pointer to a struct held in the easy handle. That's
where the current code already adds a struct now when Curl_expire() is called
if there's already a timer pending.
Thinking about it, isn't this already a problem that makes the current code
not thread-safe when threaded resolving is used?
We need to protect the Curl_expire() call in lib/hostthre.c with the mutex.
> With the in-line proposal, there is no possibility of an out-of-memory
> error, which could make error handling easier (e.g. the current tree drops
> timeouts silently if malloc fails in multi_addtimeout).
Then it would have to be truly in-line which would require a rewrite of some
> failing to delete the timer before the struct leads to a memory leak.
Not really. We always clear all pending timeouts when the handle is killed
(and when removed from the multi handle) so I see little risk of leaking
memory due to this.
-- / daniel.haxx.se
List admin: http://cool.haxx.se/list/listinfo/curl-library
Received on 2010-08-18