Infinite loop in pop3-retr

Infinite loop in pop3-retr

[tomga]

Sometimes during retrieving mail through pop3 in gnus I emacs loops infinitely in pop3-retr. It seems to be somehow connected with some buggy mail. Normally I didn't have time to investigate it thoroughly (I haven't used emacs debugger yet) but this time I think I've found out enough to make this problem fixed by someone.

Problem is inside:
      (set-buffer (process-buffer process))
      (while (not (re-search-forward "^\\.\r\n" nil t))
	(pop3-accept-process-output process)
	(goto-char start))

If I correctly read it this code assumes that in (process-buffer process) finally will contain line with dot only but this never happens probably due to information found at the end of trace of pop session to server saying:

Process POP connection broken by remote peer.

Can you propose some fix for this?

Regards
Tomasz Gajewski

PS. I had to send this from webmail so formatting may be somewhat broken.

Re: Infinite loop in pop3-retr

[Stefan Monnier]

> Sometimes during retrieving mail through pop3 in gnus I emacs loops infinitely in pop3-retr. It seems to be somehow connected with some buggy mail. Normally I didn't have time to investigate it thoroughly (I haven't used emacs debugger yet) but this time I think I've found out enough to make this problem fixed by someone.
> Problem is inside:
>       (set-buffer (process-buffer process))
>       (while (not (re-search-forward "^\\.\r\n" nil t))
> 	(pop3-accept-process-output process)
> 	(goto-char start))

> If I correctly read it this code assumes that in (process-buffer
> process) finally will contain line with dot only but this never
> happens probably due to information found at the end of trace of pop
> session to server saying:

> Process POP connection broken by remote peer.

> Can you propose some fix for this?

The while should also test whether the `process' is still alive.  Or else,
pop3-accept-process-output should somehow signal an error if `process' is
not alive any more.


        Stefan

Re: Infinite loop in pop3-retr

[Tomasz Gajewski]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Sometimes during retrieving mail through pop3 in gnus I emacs loops
>> infinitely in pop3-retr. It seems to be somehow connected with some
>> buggy mail. Normally I didn't have time to investigate it thoroughly
>> (I haven't used emacs debugger yet) but this time I think I've found
>> out enough to make this problem fixed by someone.
>>
>> Problem is inside:
>>   (set-buffer (process-buffer process))
>>   (while (not (re-search-forward "^\\.\r\n" nil t))
>>     (pop3-accept-process-output process)
>>     (goto-char start))
>
>> If I correctly read it this code assumes that in (process-buffer
>> process) finally will contain line with dot only but this never
>> happens probably due to information found at the end of trace of pop
>> session to server saying:
>
>> Process POP connection broken by remote peer.
>
>> Can you propose some fix for this?
>
> The while should also test whether the `process' is still alive.  Or
> else, pop3-accept-process-output should somehow signal an error if
> `process' is not alive any more.

I have no experience with emacs lisp programming and I don't like my
solution (but it is the only one I could find).

   (set-buffer (process-buffer process))
   (while (and (not (re-search-forward "^\\.\r\n" nil t))
               (member (process-status process)
                                       (list 'run 'stop 'open
                                             'listen 'connect)))
     (pop3-accept-process-output process)
     (goto-char start))

Firstly, I think that probably there is a simpler way to check if
process is active than by testing process status but I don't know that.

Secondly, I can't test this code right now becuase problem happens only
sometimes.

Can someone with more experience update this code (or maybe say that it
is not required)?

Should I sent a bug report with those informations somewhere (gnus bugs
list, emacs bugs list) or is it sufficient leave it here?

Regards

-- 
Tomasz Gajewski

Re: Infinite loop in pop3-retr

[Stefan Monnier]

>    (set-buffer (process-buffer process))
>    (while (and (not (re-search-forward "^\\.\r\n" nil t))
>                (member (process-status process)
>                                        (list 'run 'stop 'open
>                                              'listen 'connect)))
>      (pop3-accept-process-output process)
>      (goto-char start))

Sounds about right (your `list' can be simplified to '(run stop open
listen connect) and `memq' can be used instead of `member' tho it won't
make much of a difference).  But I'd expect the code after the `while'
loop would need to be checked to see if it handles the case where we
stopped before finding "^\\.\r\n".

> Firstly, I think that probably there is a simpler way to check if
> process is active than by testing process status but I don't know that.

Actually, no, this is the way to do it.

> Can someone with more experience update this code (or maybe say that it
> is not required)?
> Should I sent a bug report with those informations somewhere (gnus bugs
> list, emacs bugs list) or is it sufficient leave it here?

Gnus's bugs list is probably the best place, yes.


        Stefan

Re: Infinite loop in pop3-retr

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Firstly, I think that probably there is a simpler way to check if
>> process is active than by testing process status but I don't know that.
>
> Actually, no, this is the way to do it.

Which reminds me.  Instead of sprinkling all the code with

  (memq (process-status process) '(open run))

which isn't very...  pretty...  (there's probably hundreds variations of
these invocations in Gnus and the other network oriented packages)
wouldn't it be nice if we had a nice, simple function like
`process-alive-p' or something like that?  It would just basically be

(defun process-alive-p (process)
  (memq (process-status process)  
        '(run open listen connect)))
        
-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

Re: Infinite loop in pop3-retr

[Stefan Monnier]

>>> Firstly, I think that probably there is a simpler way to check if
>>> process is active than by testing process status but I don't know that.
>> Actually, no, this is the way to do it.
> Which reminds me.  Instead of sprinkling all the code with

>   (memq (process-status process) '(open run))

> which isn't very...  pretty...  (there's probably hundreds variations of
> these invocations in Gnus and the other network oriented packages)
> wouldn't it be nice if we had a nice, simple function like
> `process-alive-p' or something like that?  It would just basically be

> (defun process-alive-p (process)
>   (memq (process-status process)
>         '(run open listen connect)))
        
Sounds fine to me.  Tho I wonder if `stop' should be in this list.


        Stefan

Re: Infinite loop in pop3-retr

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> (defun process-alive-p (process)
>>   (memq (process-status process)
>>         '(run open listen connect)))
>
> Sounds fine to me.  Tho I wonder if `stop' should be in this list.

Hm...  probably...  when do the processes go into `stop' state?  And can
the process be considered alive when it's in that state?

I'll install the function.

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

Re: Infinite loop in pop3-retr

[Stefan Monnier]

>>> (defun process-alive-p (process)
>>> (memq (process-status process)
>>> '(run open listen connect)))
>> Sounds fine to me.  Tho I wonder if `stop' should be in this list.
> Hm...  probably...  when do the processes go into `stop' state?

IIUC this is the state when a process is suspended (like after a C-z).
So it can later come back to `run' state.

> And can the process be considered alive when it's in that state?

It's definitely not dead, but it's not responsive either.


        Stefan

Re: Infinite loop in pop3-retr

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> IIUC this is the state when a process is suspended (like after a C-z).
> So it can later come back to `run' state.

Right.  I included `stop' in `process-alive-p'.

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

Re: Infinite loop in pop3-retr

[Chong Yidong]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> IIUC this is the state when a process is suspended (like after a C-z).
>> So it can later come back to `run' state.
>
> Right.  I included `stop' in `process-alive-p'.

`process-alive-p' should be `process-live-p', for consistency with
buffer-live-p, frame-live-p, etc.  Could you rename it?

Re: Infinite loop in pop3-retr

[Lars Magne Ingebrigtsen]

Chong Yidong <cyd@stupidchicken.com> writes:

> `process-alive-p' should be `process-live-p', for consistency with
> buffer-live-p, frame-live-p, etc.  Could you rename it?

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

Re: Infinite loop in pop3-retr

[Lars Magne Ingebrigtsen]

Tomasz Gajewski <tomga@wp.pl> writes:

>                (member (process-status process)
>                                        (list 'run 'stop 'open
>                                              'listen 'connect)))

I've installed a variant of this, only I've made it error out instead of
taking the closing of the connection as a "I've found the end", which
might lead to losing email.

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

Re: Infinite loop in pop3-retr

[Tomasz Gajewski]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> I've installed a variant of this, ...

Great to hear this. Thank you.

-- 
Tomasz Gajewski