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
sorts!

> 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
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2010-08-18