GNU bug report logs -
#43682
28.0.50; Clean up nnimap server buffers?
Previous Next
Reported by: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Date: Mon, 28 Sep 2020 23:38:02 UTC
Severity: normal
Found in version 28.0.50
Done: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 43682 in the body.
You can then email your comments to 43682 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Mon, 28 Sep 2020 23:38:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Eric Abrahamsen <eric <at> ericabrahamsen.net>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Mon, 28 Sep 2020 23:38:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Someone noted on gnus.general that their imap connections are frequently
broken, and they end up with a lot of dead process buffers.
I'm talking to them about maybe making the keepalive timeout
configurable, but wouldn't also be tidy to clean up dead process
buffers? How does the attached patch look?
Eric
[CleanupImapBuffers.diff (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Tue, 29 Sep 2020 07:43:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 43682 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Mon, 28 Sep 2020 16:37:15 -0700, Eric Abrahamsen <eric <at> ericabrahamsen.net> said:
Eric> Someone noted on gnus.general that their imap connections are frequently
Eric> broken, and they end up with a lot of dead process buffers.
Eric> I'm talking to them about maybe making the keepalive timeout
Eric> configurable, but wouldn't also be tidy to clean up dead process
Eric> buffers? How does the attached patch look?
Eric> Eric
Eric> diff --git a/lisp/gnus/nnimap.el b/lisp/gnus/nnimap.el
Eric> index d797e893f5..7f2ebe279e 100644
Eric> --- a/lisp/gnus/nnimap.el
Eric> +++ b/lisp/gnus/nnimap.el
Eric> @@ -407,8 +407,15 @@ nnimap-keepalive
Eric> (time-subtract
Eric> now
Eric> (nnimap-last-command-time nnimap-object))))
Eric> - (ignore-errors ;E.g. "buffer foo has no process".
Eric> - (nnimap-send-command "NOOP"))))))))
Eric> + (condition-case err
Eric> + (process-send-string "NOOP")
Eric> + (error
Eric> + (if (string-search "has no process" (cdr err))
Eric> + (let ((buf (current-buffer)))
Eric> + (setq nnimap-process-buffers
Eric> + (delq buf nnimap-process-buffers))
Eric> + (kill-buffer buf))
Eric> + (signal (car err) (cdr err)))))))))))
Thatʼs not how you call process-send-string, and the
nnimap-send-command is there for a reason: it deals with imap sequence
numbers.
Also I donʼt think 'has no process' is the only error
'process-send-string' can signal. I see at least 'not running' and
'Output file descriptor.*is closed'
Robert
--
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Tue, 29 Sep 2020 14:52:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 43682 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> - (ignore-errors ;E.g. "buffer foo has no process".
> - (nnimap-send-command "NOOP"))))))))
> + (condition-case err
> + (process-send-string "NOOP")
> + (error
As Robert notes, you can't do that; it'll mess up the machiner.
> + (if (string-search "has no process" (cdr err))
> + (let ((buf (current-buffer)))
> + (setq nnimap-process-buffers
> + (delq buf nnimap-process-buffers))
> + (kill-buffer buf))
> + (signal (car err) (cdr err)))))))))))
But why look for a string? You can just check whether the process is
dead or not.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Tue, 29 Sep 2020 18:26:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 43682 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> - (ignore-errors ;E.g. "buffer foo has no process".
>> - (nnimap-send-command "NOOP"))))))))
>> + (condition-case err
>> + (process-send-string "NOOP")
>> + (error
>
> As Robert notes, you can't do that; it'll mess up the machiner.
That was a complete brain-o. I must have actually gone and typed that,
and I don't know why.
>> + (if (string-search "has no process" (cdr err))
>> + (let ((buf (current-buffer)))
>> + (setq nnimap-process-buffers
>> + (delq buf nnimap-process-buffers))
>> + (kill-buffer buf))
>> + (signal (car err) (cdr err)))))))))))
>
> But why look for a string? You can just check whether the process is
> dead or not.
Yes, I could have thought this through a little more...
Do we need to manipulate `nnimap-connection-alist', as
`nnimap-find-connection' does? I've never understood what that variable
is actually for. It's a defvoo, so it has a separate value per-server,
but each server's only got one active process buffer anyway. What does
it tell us that we can't get from nnimap-process-buffers (and those
buffers' local values of `nnimap-object')?
Thanks to you both,
Eric
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Wed, 30 Sep 2020 01:20:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 43682 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> Do we need to manipulate `nnimap-connection-alist', as
> `nnimap-find-connection' does? I've never understood what that variable
> is actually for. It's a defvoo, so it has a separate value per-server,
> but each server's only got one active process buffer anyway.
They do? I thought async prefetching opened a second connection. Is
that in nnimap only?
And... was there something about sieve-mode having its own connection?
I forget.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Wed, 30 Sep 2020 01:22:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 43682 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> They do? I thought async prefetching opened a second connection. Is
> that in nnimap only?
Sorry; I meant "nntp only".
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Wed, 30 Sep 2020 21:50:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 43682 <at> debbugs.gnu.org (full text, mbox):
On 09/30/20 03:18 AM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> Do we need to manipulate `nnimap-connection-alist', as
>> `nnimap-find-connection' does? I've never understood what that variable
>> is actually for. It's a defvoo, so it has a separate value per-server,
>> but each server's only got one active process buffer anyway.
>
> They do? I thought async prefetching opened a second connection. Is
> that in nnimap only?
So that's what it's for! That's a mystery solved. It appears that only
nntp is asynchronous: at least, it's the only backend that implements
`gnus-asynchronous-p'.
nntp.el also contains `nntp-async-{wait,stop-trigger}', but as far as I
can tell that has nothing to do with gnus-async.el stuff (?). That's the
code that appears to make use of "extra processes per server", and
nnimap doesn't have any of that.
> And... was there something about sieve-mode having its own connection?
> I forget.
Not so far as I can tell. sieve.el and sieve-mode.el don't have anything
to do with Gnus, and gnus-sieve.el is just about building sieve scripts
from Gnus splitting rules. (Now that you mention it, it would be nice if
there were a command to edit an IMAP server's sieve scripts from the
*Server* buffer.)
I went back through nnimap.el as carefully as I could. It's
`nnimap-find-connection' and `nnimap-open-connection' that end up making
use of `nnimap-connection-alist', but both of them only ever use
`nntp-server-buffer' as the alist key. Unless that's been sneakily
let-bound to something different someplace, the
`nnimap-connection-alist' doesn't appear to do anything.
Also, `nnimap-make-process-buffer' sets a buffer-local version of
`after-change-functions' to nil -- this looks a bit like how nntp
handles its async code, but it isn't used in nnimap. My guess is that
someone set out to implement async for nnimap, but never quite finished
it.
I'm inclined to delete the alist altogether, but if we keep it at least
I can give it a docstring.
Eric
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Thu, 01 Oct 2020 01:31:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 43682 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> So that's what it's for! That's a mystery solved. It appears that only
> nntp is asynchronous: at least, it's the only backend that implements
> `gnus-asynchronous-p'.
Aha. nnimap should, too.
> nntp.el also contains `nntp-async-{wait,stop-trigger}', but as far as I
> can tell that has nothing to do with gnus-async.el stuff (?). That's the
> code that appears to make use of "extra processes per server", and
> nnimap doesn't have any of that.
Yes, that's a separate thing...
> Not so far as I can tell. sieve.el and sieve-mode.el don't have anything
> to do with Gnus, and gnus-sieve.el is just about building sieve scripts
> from Gnus splitting rules. (Now that you mention it, it would be nice if
> there were a command to edit an IMAP server's sieve scripts from the
> *Server* buffer.)
Right. I think there was once some talk about this stuff, but oy vey
the years...
> I'm inclined to delete the alist altogether, but if we keep it at least
> I can give it a docstring.
No, we should keep it and implement the stuff for gnus-asynchronous-p in
nnimap, too.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Thu, 01 Oct 2020 05:10:01 GMT)
Full text and
rfc822 format available.
Message #29 received at 43682 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 10/01/20 03:30 AM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> So that's what it's for! That's a mystery solved. It appears that only
>> nntp is asynchronous: at least, it's the only backend that implements
>> `gnus-asynchronous-p'.
>
> Aha. nnimap should, too.
>
>> nntp.el also contains `nntp-async-{wait,stop-trigger}', but as far as I
>> can tell that has nothing to do with gnus-async.el stuff (?). That's the
>> code that appears to make use of "extra processes per server", and
>> nnimap doesn't have any of that.
>
> Yes, that's a separate thing...
>
>> Not so far as I can tell. sieve.el and sieve-mode.el don't have anything
>> to do with Gnus, and gnus-sieve.el is just about building sieve scripts
>> from Gnus splitting rules. (Now that you mention it, it would be nice if
>> there were a command to edit an IMAP server's sieve scripts from the
>> *Server* buffer.)
>
> Right. I think there was once some talk about this stuff, but oy vey
> the years...
I'll add it to my own todo list, where it can likewise languish for
years.
>> I'm inclined to delete the alist altogether, but if we keep it at least
>> I can give it a docstring.
>
> No, we should keep it and implement the stuff for gnus-asynchronous-p in
> nnimap, too.
It's possible (I'm not claiming to understand all the code) that all we
would need to do is fix `gnus-async-wait-for-article' to replace its
calls to `nntp-find-connection' and `nntp-accept-process-output' with
something generalized. Those two functions deal with directly with
`nntp-connection-alist', so we'd need something that would do the
equivalent with `nnimap-connection-alist'.
Anyway, in the interest of completing this far less ambitious patch: if
the nnimap connection has timed out, we should remove this connection
from `nnimap-connection-alist', so this version of the patch does that.
If async has opened a second connection, I guess we should leave that
alone, though I don't have too much confidence that the whole process
will recover gracefully from the main connection dying...
Eric
[CleanupImapBuffers.diff (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Thu, 01 Oct 2020 16:02:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 43682 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> It's possible (I'm not claiming to understand all the code) that all we
> would need to do is fix `gnus-async-wait-for-article' to replace its
> calls to `nntp-find-connection' and `nntp-accept-process-output' with
> something generalized. Those two functions deal with directly with
> `nntp-connection-alist', so we'd need something that would do the
> equivalent with `nnimap-connection-alist'.
Yup.
> Anyway, in the interest of completing this far less ambitious patch: if
> the nnimap connection has timed out, we should remove this connection
> from `nnimap-connection-alist', so this version of the patch does that.
> If async has opened a second connection, I guess we should leave that
> alone, though I don't have too much confidence that the whole process
> will recover gracefully from the main connection dying...
Well, the connections are separate, and there's all kinds of reasons for
the server to close a connection, so...
> + (unless (memq (process-status (get-buffer-process buffer))
> + '(open run))
Aka `process-live-p'.
Otherwise looks fine to me (but I haven't tested the code).
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Thu, 01 Oct 2020 17:26:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 43682 <at> debbugs.gnu.org (full text, mbox):
On 10/01/20 18:01 PM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> It's possible (I'm not claiming to understand all the code) that all we
>> would need to do is fix `gnus-async-wait-for-article' to replace its
>> calls to `nntp-find-connection' and `nntp-accept-process-output' with
>> something generalized. Those two functions deal with directly with
>> `nntp-connection-alist', so we'd need something that would do the
>> equivalent with `nnimap-connection-alist'.
>
> Yup.
This is something I wouldn't want to tackle until we have generic
functions.
>> Anyway, in the interest of completing this far less ambitious patch: if
>> the nnimap connection has timed out, we should remove this connection
>> from `nnimap-connection-alist', so this version of the patch does that.
>> If async has opened a second connection, I guess we should leave that
>> alone, though I don't have too much confidence that the whole process
>> will recover gracefully from the main connection dying...
>
> Well, the connections are separate, and there's all kinds of reasons for
> the server to close a connection, so...
>
>> + (unless (memq (process-status (get-buffer-process buffer))
>> + '(open run))
>
> Aka `process-live-p'.
I forgot we have that!
> Otherwise looks fine to me (but I haven't tested the code).
Okay, I'll run this for a bit, as well.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Mon, 11 Oct 2021 12:54:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 43682 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> On 10/01/20 18:01 PM, Lars Ingebrigtsen wrote:
>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>
>>> It's possible (I'm not claiming to understand all the code) that all we
>>> would need to do is fix `gnus-async-wait-for-article' to replace its
>>> calls to `nntp-find-connection' and `nntp-accept-process-output' with
>>> something generalized. Those two functions deal with directly with
>>> `nntp-connection-alist', so we'd need something that would do the
>>> equivalent with `nnimap-connection-alist'.
>>
>> Yup.
>
> This is something I wouldn't want to tackle until we have generic
> functions.
>
>>> Anyway, in the interest of completing this far less ambitious patch: if
>>> the nnimap connection has timed out, we should remove this connection
>>> from `nnimap-connection-alist', so this version of the patch does that.
>>> If async has opened a second connection, I guess we should leave that
>>> alone, though I don't have too much confidence that the whole process
>>> will recover gracefully from the main connection dying...
>>
>> Well, the connections are separate, and there's all kinds of reasons for
>> the server to close a connection, so...
>>
>>> + (unless (memq (process-status (get-buffer-process buffer))
>>> + '(open run))
>>
>> Aka `process-live-p'.
>
> I forgot we have that!
>
>> Otherwise looks fine to me (but I haven't tested the code).
>
> Okay, I'll run this for a bit, as well.
Any news here? Should the fix be installed?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Mon, 11 Oct 2021 14:43:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 43682 <at> debbugs.gnu.org (full text, mbox):
Stefan Kangas <stefan <at> marxist.se> writes:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> On 10/01/20 18:01 PM, Lars Ingebrigtsen wrote:
>>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>>
>>>> It's possible (I'm not claiming to understand all the code) that all we
>>>> would need to do is fix `gnus-async-wait-for-article' to replace its
>>>> calls to `nntp-find-connection' and `nntp-accept-process-output' with
>>>> something generalized. Those two functions deal with directly with
>>>> `nntp-connection-alist', so we'd need something that would do the
>>>> equivalent with `nnimap-connection-alist'.
>>>
>>> Yup.
>>
>> This is something I wouldn't want to tackle until we have generic
>> functions.
>>
>>>> Anyway, in the interest of completing this far less ambitious patch: if
>>>> the nnimap connection has timed out, we should remove this connection
>>>> from `nnimap-connection-alist', so this version of the patch does that.
>>>> If async has opened a second connection, I guess we should leave that
>>>> alone, though I don't have too much confidence that the whole process
>>>> will recover gracefully from the main connection dying...
>>>
>>> Well, the connections are separate, and there's all kinds of reasons for
>>> the server to close a connection, so...
>>>
>>>> + (unless (memq (process-status (get-buffer-process buffer))
>>>> + '(open run))
>>>
>>> Aka `process-live-p'.
>>
>> I forgot we have that!
>>
>>> Otherwise looks fine to me (but I haven't tested the code).
>>
>> Okay, I'll run this for a bit, as well.
>
> Any news here? Should the fix be installed?
Hmm, I found a couple of typos in it, then I ran it for a bit, then got
distracted and backed it out in favor of something else, and then...
I think it should be okay to push. I'll try a bit of explicit testing,
then push later today. Thanks for the reminder!
Reply sent
to
Eric Abrahamsen <eric <at> ericabrahamsen.net>
:
You have taken responsibility.
(Tue, 12 Oct 2021 20:51:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Eric Abrahamsen <eric <at> ericabrahamsen.net>
:
bug acknowledged by developer.
(Tue, 12 Oct 2021 20:51:02 GMT)
Full text and
rfc822 format available.
Message #46 received at 43682-done <at> debbugs.gnu.org (full text, mbox):
On 10/11/21 05:53 AM, Stefan Kangas wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> On 10/01/20 18:01 PM, Lars Ingebrigtsen wrote:
>>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>>
>>>> It's possible (I'm not claiming to understand all the code) that all we
>>>> would need to do is fix `gnus-async-wait-for-article' to replace its
>>>> calls to `nntp-find-connection' and `nntp-accept-process-output' with
>>>> something generalized. Those two functions deal with directly with
>>>> `nntp-connection-alist', so we'd need something that would do the
>>>> equivalent with `nnimap-connection-alist'.
>>>
>>> Yup.
>>
>> This is something I wouldn't want to tackle until we have generic
>> functions.
>>
>>>> Anyway, in the interest of completing this far less ambitious patch: if
>>>> the nnimap connection has timed out, we should remove this connection
>>>> from `nnimap-connection-alist', so this version of the patch does that.
>>>> If async has opened a second connection, I guess we should leave that
>>>> alone, though I don't have too much confidence that the whole process
>>>> will recover gracefully from the main connection dying...
>>>
>>> Well, the connections are separate, and there's all kinds of reasons for
>>> the server to close a connection, so...
>>>
>>>> + (unless (memq (process-status (get-buffer-process buffer))
>>>> + '(open run))
>>>
>>> Aka `process-live-p'.
>>
>> I forgot we have that!
>>
>>> Otherwise looks fine to me (but I haven't tested the code).
>>
>> Okay, I'll run this for a bit, as well.
>
> Any news here? Should the fix be installed?
Okay, I've pushed a commit that fixes nearly all of the issue. There are
still some mysterious circumstances under which a single dead imap
connection buffer remains in `nnimap-process-buffers', and I don't know
how it gets there. But it's better than 50+ dead connection buffers, so
I'm going to close this for now.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43682
; Package
emacs
.
(Tue, 12 Oct 2021 21:35:02 GMT)
Full text and
rfc822 format available.
Message #49 received at 43682-done <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> Okay, I've pushed a commit that fixes nearly all of the issue. There are
> still some mysterious circumstances under which a single dead imap
> connection buffer remains in `nnimap-process-buffers', and I don't know
> how it gets there. But it's better than 50+ dead connection buffers, so
> I'm going to close this for now.
Thanks!
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Wed, 10 Nov 2021 12:24:12 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 161 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.