bug#49570: 28.0.50; url-http proxy connection-wait

bug#49570: 28.0.50; url-http proxy connection-wait

[Madhu]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]


When accessing https://melpa.org (an nginx webserver) through a
http-proxy (by setting url-proxy-services, say to privoxy), url-http
hangs when parsing headers. apparently the connection is dropped with
emacs master. I'm on this commit on master

GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2021-06-15
 794ec934a76d0647a72b7be32e20dc5b95e5ec11

#ELISP> (setq url-proxy-services '(("http" . "localhost:8118") ("https" . "localhost:8118")))
ELISP> (url-retrieve-synchronously (url-generic-parse-url "https://melpa.org/"))
#<buffer  *http melpa.org:443*>

And the buffer looks like

#+BEGIN_EXAMPLE

Process melpa.org connection broken by remote peer
HTTP/1.1 200 OK
Server: nginx
Date: Thu, 15 Jul 2021 09:41:12 GMT
Content-Type: text/html; charset=utf-8
Last-Modified: Wed, 10 Feb 2021 23:56:44 GMT
Connection: close
Vary: Accept-Encoding
ETag: W/"602472bc-d92"
Strict-Transport-Security: max-age=15768000
Content-Encoding: gzip
#+END_EXAMPLE

The connection is broken and the "\nProcess HOST connection broken by
remote peer\n" line in the buffer means the spins without parsing the
header.


Now proxied requests are asynchronous.

If I (setf (url-asynchronous url) t) in lisp/url/url.el:
(url-retrieve-internal), as per the appended patch, then eventually
make-network-process gets called with a :nowait t, and the connection
pulls through.

But I'm not sure if this is the right thing or if the problem is
elsewhere as this problem doesn't happen on older emacs - say from
2020-10-15.

Do others see this problem?

[BTW I haven't seen paul eggert on the lists lately, any word if he is
still too busy for emacs?]



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: proxy connections non-blocking --]
[-- Type: text/x-diff, Size: 551 bytes --]

diff --git a/lisp/url/url.el b/lisp/url/url.el
index 00c5740cf8..e40a4ecc29 100644
--- a/lisp/url/url.el
+++ b/lisp/url/url.el
@@ -211,8 +211,9 @@ url-retrieve-internal
 	(buffer nil)
 	(asynch (url-scheme-get-property (url-type url) 'asynchronous-p)))
     (if url-using-proxy
-	(setq asynch t
-	      loader #'url-proxy))
+	(progn (setq asynch t
+	             loader #'url-proxy)
+               (setf (url-asynchronous url) t)))
     (if asynch
 	(let ((url-current-object url))
 	  (setq buffer (funcall loader url callback cbargs)))
-- 
2.31.0

bug#49570: 28.0.50; url-http proxy connection-wait

[Lars Ingebrigtsen]

Madhu <enometh@meer.net> writes:

> When accessing https://melpa.org (an nginx webserver) through a
> http-proxy (by setting url-proxy-services, say to privoxy), url-http
> hangs when parsing headers. apparently the connection is dropped with
> emacs master. I'm on this commit on master
>
> GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
>  version 1.16.0, Xaw3d scroll bars) of 2021-06-15
>  794ec934a76d0647a72b7be32e20dc5b95e5ec11

Could you try updating the branch?  Mattias pushed a connection logic
fix a couple of days ago that may (or may not) change things in this
area.

> If I (setf (url-asynchronous url) t) in lisp/url/url.el:
> (url-retrieve-internal), as per the appended patch, then eventually
> make-network-process gets called with a :nowait t, and the connection
> pulls through.
>
> But I'm not sure if this is the right thing or if the problem is
> elsewhere as this problem doesn't happen on older emacs - say from
> 2020-10-15.

It'd be interesting if you could bisect to the commit that broke this.

> [BTW I haven't seen paul eggert on the lists lately, any word if he is
> still too busy for emacs?]

He was here just the other day.  :-)

> -	(setq asynch t
> -	      loader #'url-proxy))
> +	(progn (setq asynch t
> +	             loader #'url-proxy)
> +               (setf (url-asynchronous url) t)))

Hm...  Looking at the code, I think this patch looks correct.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49570: 28.0.50; url-http proxy connection-wait

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> -	(setq asynch t
>> -	      loader #'url-proxy))
>> +	(progn (setq asynch t
>> +	             loader #'url-proxy)
>> +               (setf (url-asynchronous url) t)))
>
> Hm...  Looking at the code, I think this patch looks correct.

So I applied the patch to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49570: 28.0.50; url-http proxy connection-wait

[Madhu]

*  Lars Ingebrigtsen <larsi@gnus.org> <87lf62l37y.fsf@gnus.org>
Wrote on Mon, 19 Jul 2021 17:42:41 +0200

> So I applied the patch to Emacs 28.

Thanks.  (I'm afraid I haven't found the time to bisect it, and
running native-comp is a disincentive to change HEAD (with debug it
takes 100s of GB) but maybe i can still try to bisect after I update
-- for curiosity's sake)

bug#49570: 28.0.50; url-http proxy connection-wait

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Madhu <enometh@meer.net> writes:

> *  Lars Ingebrigtsen <larsi@gnus.org> <87lf62l37y.fsf@gnus.org>
> Wrote on Mon, 19 Jul 2021 17:42:41 +0200
>
>> So I applied the patch to Emacs 28.
>
> Thanks.  (I'm afraid I haven't found the time to bisect it, and
> running native-comp is a disincentive to change HEAD (with debug it
> takes 100s of GB) but maybe i can still try to bisect after I update
> -- for curiosity's sake)

native-comp is merged into master since a while.

Actually I thought I removed feature/native-comp, is it still in place?

Regards

  Andrea

bug#49570: 28.0.50; url-http proxy connection-wait

[Lars Ingebrigtsen]

Andrea Corallo <akrl@sdf.org> writes:

> Actually I thought I removed feature/native-comp, is it still in place?

I think it's gone from Savannah, but if I remember correctly, that
doesn't make the branch go away in other repos?  And indeed:

larsi@elva:~/src/emacs/trunk$ git branch -a | grep native
  remotes/origin/feature/native-comp
  remotes/origin/feature/native-comp-macos-fixes
  remotes/origin/scratch/native-comp-cl
  remotes/origin/scratch/native-comp-eln-caches
  remotes/origin/scratch/native-comp-gcc-driver-options
  remotes/origin/scratch/native-comp-macos-43532

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49570: 28.0.50; url-http proxy connection-wait

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Andrea Corallo <akrl@sdf.org> writes:
>
>> Actually I thought I removed feature/native-comp, is it still in place?
>
> I think it's gone from Savannah, but if I remember correctly, that
> doesn't make the branch go away in other repos?  And indeed:
>
> larsi@elva:~/src/emacs/trunk$ git branch -a | grep native
>   remotes/origin/feature/native-comp
>   remotes/origin/feature/native-comp-macos-fixes
>   remotes/origin/scratch/native-comp-cl
>   remotes/origin/scratch/native-comp-eln-caches
>   remotes/origin/scratch/native-comp-gcc-driver-options
>   remotes/origin/scratch/native-comp-macos-43532

I see, that's annoying but I guess we have no workaround for that :/

Thanks for checking.

  Andrea

bug#49570: 28.0.50; url-http proxy connection-wait

[Lars Ingebrigtsen]

Andrea Corallo <akrl@sdf.org> writes:

> I see, that's annoying but I guess we have no workaround for that :/

Let's see...  The magical git command seems to be:

git fetch origin --prune

And that did make the native-comp branch disappear locally.  But it's
something each user has to do.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49570: 28.0.50; url-http proxy connection-wait

[Eli Zaretskii]

> Cc: Madhu <enometh@meer.net>, 49570@debbugs.gnu.org
> Date: Tue, 20 Jul 2021 15:13:01 +0000
> From:  Andrea Corallo via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > Andrea Corallo <akrl@sdf.org> writes:
> >
> >> Actually I thought I removed feature/native-comp, is it still in place?
> >
> > I think it's gone from Savannah, but if I remember correctly, that
> > doesn't make the branch go away in other repos?  And indeed:
> >
> > larsi@elva:~/src/emacs/trunk$ git branch -a | grep native
> >   remotes/origin/feature/native-comp
> >   remotes/origin/feature/native-comp-macos-fixes
> >   remotes/origin/scratch/native-comp-cl
> >   remotes/origin/scratch/native-comp-eln-caches
> >   remotes/origin/scratch/native-comp-gcc-driver-options
> >   remotes/origin/scratch/native-comp-macos-43532
> 
> I see, that's annoying but I guess we have no workaround for that :/

The workaround is to tell people to say

   git remote prune origin

from time to time.

bug#49570: 28.0.50; url-http proxy connection-wait

[Gregory Heytings]

>>>> Actually I thought I removed feature/native-comp, is it still in 
>>>> place?
>>>
>>> I think it's gone from Savannah, but if I remember correctly, that 
>>> doesn't make the branch go away in other repos?
>>
>> I see, that's annoying but I guess we have no workaround for that :/
>
> The workaround is to tell people to say
>
>   git remote prune origin
>
> from time to time.
>

No, the workaround is to tell people to set

git config remote.origin.prune true

bug#49570: 28.0.50; url-http proxy connection-wait

[Gregory Heytings]

>>>>> Actually I thought I removed feature/native-comp, is it still in 
>>>>> place?
>>>> 
>>>> I think it's gone from Savannah, but if I remember correctly, that 
>>>> doesn't make the branch go away in other repos?
>>> 
>>> I see, that's annoying but I guess we have no workaround for that :/
>> 
>> The workaround is to tell people to say
>>
>>   git remote prune origin
>> 
>> from time to time.
>
> No, the workaround is to tell people to set
>
> git config remote.origin.prune true
>

(Or, of course, to add a "git_config remote.origin.prune true" in 
autogen.sh.)

bug#49570: 28.0.50; url-http proxy connection-wait

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Gregory Heytings <gregory@heytings.org> writes:

>>>>>> Actually I thought I removed feature/native-comp, is it still in place?
>>>>> I think it's gone from Savannah, but if I remember correctly, that doesn't
>>>>> make the branch go away in other repos?
>>>> I see, that's annoying but I guess we have no workaround for that :/
>>> The workaround is to tell people to say
>>>
>>>   git remote prune origin
>>> from time to time.
>>
>> No, the workaround is to tell people to set
>>
>> git config remote.origin.prune true
>
> (Or, of course, to add a "git_config remote.origin.prune true" in autogen.sh.)

If pruning loses data it shouldn't be enabled by default in all
checkouts, e.g. if a branch is accidentally deleted by someone else then
simply fetching shouldn't propagate that deletion for others.  So IMO
the developer should decide when and how to enable this.  (Unless I've
misunderstood something.)

-- 
Basil

bug#49570: 28.0.50; url-http proxy connection-wait

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Eli Zaretskii <eliz@gnu.org>,  larsi@gnus.org,  49570@debbugs.gnu.org,
>   enometh@meer.net,  Andrea Corallo <akrl@sdf.org>
> Date: Wed, 21 Jul 2021 13:46:39 +0100
> 
> Gregory Heytings <gregory@heytings.org> writes:
> 
> >> No, the workaround is to tell people to set
> >>
> >> git config remote.origin.prune true
> >
> > (Or, of course, to add a "git_config remote.origin.prune true" in autogen.sh.)
> 
> If pruning loses data it shouldn't be enabled by default in all
> checkouts, e.g. if a branch is accidentally deleted by someone else then
> simply fetching shouldn't propagate that deletion for others.

Of course.  Don't worry, we won't do that.

bug#49570: 28.0.50; url-http proxy connection-wait

[Gregory Heytings]

>
> If pruning loses data it shouldn't be enabled by default in all 
> checkouts, e.g. if a branch is accidentally deleted by someone else then 
> simply fetching shouldn't propagate that deletion for others.  So IMO 
> the developer should decide when and how to enable this.  (Unless I've 
> misunderstood something.)
>

Pruning a branch does not lose data AFAIK, it only removes the pointer to 
the last commit of that branch.

bug#49570: 28.0.50; url-http proxy connection-wait

[Gregory Heytings]

>> If pruning loses data it shouldn't be enabled by default in all 
>> checkouts, e.g. if a branch is accidentally deleted by someone else 
>> then simply fetching shouldn't propagate that deletion for others.
>
> Of course.  Don't worry, we won't do that.
>

Why on earth won't you do that?  Deleting a branch (or removing a tag) 
does *not* lose any data / commit, it only removes a pointer / label to a 
particular commit.  For example anyone can still access the commits of the 
feature/native-comp branch with git checkout fa65c044f2.  Should someone 
accidentally delete a branch, it is easy to create it again.

Setting this config variable is much better than telling people to say 
"git remote prune origin" from time to time as you suggested, it does the 
same, except that it happens automatically.

bug#49570: 28.0.50; url-http proxy connection-wait

[Eli Zaretskii]

> Date: Thu, 22 Jul 2021 07:30:30 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: "Basil L. Contovounesios" <contovob@tcd.ie>, enometh@meer.net, 
>     49570@debbugs.gnu.org, larsi@gnus.org, akrl@sdf.org
> 
> 
> >> If pruning loses data it shouldn't be enabled by default in all 
> >> checkouts, e.g. if a branch is accidentally deleted by someone else 
> >> then simply fetching shouldn't propagate that deletion for others.
> >
> > Of course.  Don't worry, we won't do that.
> 
> Why on earth won't you do that?

Because it isn't our place to second-guess when the user wants to
purge his/her local Git repository.

bug#49570: 28.0.50; url-http proxy connection-wait

[Gregory Heytings]

>> Why on earth won't you do that?
>
> Because it isn't our place to second-guess when the user wants to purge 
> his/her local Git repository.
>

The word "prune" or "purge" are perhaps a bit unfortunate, they seem to 
imply that something serious is happening (as Basil thought).  Once again 
no actual data is removed, the only thing that is removed is a pointer to 
a particular commit.  Two people (Lars and Andrea) regretted upthread that 
removing a branch does not remove them in local repositories, and I 
remember at least one recent incident in which someone was using the 
native-comp branch after it had been removed.  Which is why I suggested to 
set that configuration variable... but I preach in the wilderness.

bug#49570: 28.0.50; url-http proxy connection-wait

[Eli Zaretskii]

> Date: Thu, 22 Jul 2021 09:10:41 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: contovob@tcd.ie, enometh@meer.net, 49570@debbugs.gnu.org, larsi@gnus.org, 
>     akrl@sdf.org
> 
> >> Why on earth won't you do that?
> >
> > Because it isn't our place to second-guess when the user wants to purge 
> > his/her local Git repository.
> >
> 
> The word "prune" or "purge" are perhaps a bit unfortunate, they seem to 
> imply that something serious is happening (as Basil thought).

We are miscommunicating.  Regardless of whether the loss of data is or
isn't significant, it is impolite, to say the least, for the project
to force users to do anything in their local clone that is not
absolutely necessary.  In particular, whether a branch should or
shouldn't be deleted in the local clone is for the user to decide, not
for us.  So we should not do that.

> Two people (Lars and Andrea) regretted upthread that removing a
> branch does not remove them in local repositories, and I remember at
> least one recent incident in which someone was using the native-comp
> branch after it had been removed.  Which is why I suggested to set
> that configuration variable... but I preach in the wilderness.

People who want this automatic pruning can configure that in their
personal ~/.gitconfig.  So the information about doing that
automatically is not lost; thank you for posting it.