bug#18116: 24.3.92; url-http calls CALLBACK recursively with malformed CBARGS if the former calls `delete-process'

[Dmitry Gutov <dgutov@yandex.ru>]

On 09/10/2014 06:24 AM, Stefan Monnier wrote:

> The problem is that calling `delete-process' may run the sentinel (and
> since this is code run from the sentinel, it may end up calling the
> sentinel recursively).

I can observe a definite change in behavior, which I cannot reason about 
(when connection fails, the external process dies in Emacs 24.3, but 
still lives for some time in 24.4). Someone else should judge whether 
this is a bug or an intended change.

> So if you don't want the sentinel to be called recursively, you'll want
> to (set-process-sentinel url-http-process nil) before calling
> delete-process

Yes, that's more or less what we did in Tern: 
https://github.com/marijnh/tern/commit/21245d5b901e6dc9cfb7c8ea55220a11104a5efc

>> If the callback expects STATUS to contain some specific data structure,
>> that can cause breakage, see https://github.com/marijnh/tern/issues/350
>> for an example.
>
> The format looks normal: the STATUS is expected to be a plist holding
> the "history" of the connection.  It can contain various ":error FOO"
> and ":redirect BAR" entries.

Indeed. Looks like my misunderstanding stemmed from Tern not handling 
this value exactly right: it expected STATUS to have a certain length in 
case of an error, and when called recursively, the callback received 
STATUS of different length, prepended with new history.

I guess there's no bug there, then. Sorry.

> I think the problem comes from a doc error, where url-http points to
> url-retrieve for the doc of CBARGS, whereas it uses the format of
> url-retrieve-internal instead.

Guess so. And both `url-http' and `url-retrieve-internal' are pretty 
confusing when it comes to describing the arguments that will be passed 
to CALLBACK.

To me, "When retrieval is completed, execute the function CALLBACK, 
using the arguments listed in CBARGS." means that it will be called 
exactly with the value of CBARGS passed to `url-http', whereas instead 
the list gets prepended with stuff before it's passed to CALLBACK.