[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

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



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.