Re: [Libevent-users] RPC: patch for cancelling requests

[Christophe Fillot <cf@xxxxxx>]

Nick Mathewson wrote:

Hello Nick,
> It looks plausible, though I haven't had a chance to review it too
> closely yet.  I would like to have unit tests for this particular case
> too, if at all possible.
No problem, I'll try to make some examples soon.

> One thing to watch out for in this code is the fact that the "next"
> pointer in the request is apparently used for keeping track of the
> request's neighbors on any of the requests list and the
> started_requests list, so it's important to make sure when adding a
> request to one of these lists, it isn't already on the other one.
> (I'm not sure whether the code already makes sure of this; I wish the
> existing evprc code were better about documenting its invariants.)
In fact, a request is pushed in "started_requests" list by
evrpc_schedule_request(), which assumes the request has been
dequeued from the "requests" list (this is already the case in the
original code, by evrpc_pool_schedule()).
So, a request cannot be in both lists, that's why I re-used the
"next" field.

> The new "aborted" status code makes me slightly twitchy.  Adding a new
> status code like this means that, potentially, old callbacks that
> don't know about the new callback will stop working.  I think that
> it's okay in this one case, since the only way to get the new status
> code (if I'm understanding you right) is to free an evrpc instance
> with multiple pending requests, which would not have worked before at
> all... 
Indeed, you cannot get the new status code except if you want to
call evrpc_pool_free() while requests are pending. Without the patch,
evrpc_pool_free() frees evhttp_connections (and HTTP requests attached
to them), but the evrpc_request_wrapper structure is not freed. The
problem is that this structure uses a timer (not deleted too), which
calls evrpc_request_timeout() when expired. Unfortunately,
evrpc_request_timeout() expects the EvHTTP request to be valid
whereas it has been freed before by evrpc_pool_free().

Christophe

***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.