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

Previous Next

Full log

Message #17 received at 18116 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 18116 <at> debbugs.gnu.org
Subject: Re: bug#18116: 24.3.92; url-http calls CALLBACK recursively with
 malformed CBARGS if the former calls `delete-process'
Date: Thu, 11 Sep 2014 05:52:33 +0400

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.