* Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) @ 2004-05-30 22:34 Kim F. Storm 2004-05-30 23:09 ` Noah Friedman 0 siblings, 1 reply; 16+ messages in thread From: Kim F. Storm @ 2004-05-30 22:34 UTC (permalink / raw) Cc: emacs-devel Hi Noah, I have very severe problems fetching mail from my POP3 mailboxes after the following change was installed: 2004-05-28 Noah Friedman <friedman@splode.com> * process.c (Fdelete_process): Do not call remove_process. In essense, Gnus manages to fetch one or two articles and then it stalls, in some loop with accept-process-output and searching for \r\n in the buffer. Undoing the above change makes it work again. What (real world) problem was that change supposed to fix ? Probably it must be fixed in another way that doesn't break existing code. ++kfs ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-05-30 22:34 Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) Kim F. Storm @ 2004-05-30 23:09 ` Noah Friedman 2004-05-31 21:06 ` Kim F. Storm 0 siblings, 1 reply; 16+ messages in thread From: Noah Friedman @ 2004-05-30 23:09 UTC (permalink / raw) Cc: emacs-devel >I have very severe problems fetching mail from my POP3 mailboxes after >the following change was installed: > >2004-05-28 Noah Friedman <friedman@splode.com> > > * process.c (Fdelete_process): Do not call remove_process. > >In essense, Gnus manages to fetch one or two articles and then it stalls, >in some loop with accept-process-output and searching for \r\n in the buffer. > >Undoing the above change makes it work again. > >What (real world) problem was that change supposed to fix ? > >Probably it must be fixed in another way that doesn't break existing code. Damn. I thought I had checked fairly carefully that nothing would be affected since the call to remove_process happens anyway eventually. I left a comment in the source that explained why I removed the call. In summary, calling remove_process there does 2 things wrong: (1) network process objects are removed from Vprocess_alist even if delete-exited-processes is nil (2) network processes never have their sentinel called a final time Fdelete_process is called for network processes via kill_buffer_processes, so the mechanics of cleaning up network processes is different than for child processes, which are just signalled. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-05-30 23:09 ` Noah Friedman @ 2004-05-31 21:06 ` Kim F. Storm 2004-05-31 21:34 ` Noah Friedman 0 siblings, 1 reply; 16+ messages in thread From: Kim F. Storm @ 2004-05-31 21:06 UTC (permalink / raw) Cc: emacs-devel Noah Friedman <friedman@splode.com> writes: > I left a comment in the source that explained why I removed the call. In > summary, calling remove_process there does 2 things wrong: > > (1) network process objects are removed from Vprocess_alist even if > delete-exited-processes is nil The doc string for delete-process says: Delete PROCESS: kill it and forget about it immediately. To me 'immediately' means that the value of delete-exited-processes does not apply here. So I don't agree this is an error. > (2) network processes never have their sentinel called a final time Right, that's a real bug. The following patch should fix that (after undoing your change). *** process.c 24 May 2004 09:49:49 +0200 1.429 --- process.c 31 May 2004 22:28:55 +0200 *************** *** 764,769 **** --- 764,770 ---- { XPROCESS (process)->status = Fcons (Qexit, Fcons (make_number (0), Qnil)); XSETINT (XPROCESS (process)->tick, ++process_tick); + status_notify (); } else if (XINT (XPROCESS (process)->infd) >= 0) { Unless you see problems with this, I'll install the patch tomorrow. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-05-31 21:06 ` Kim F. Storm @ 2004-05-31 21:34 ` Noah Friedman 2004-06-02 3:43 ` Richard Stallman 0 siblings, 1 reply; 16+ messages in thread From: Noah Friedman @ 2004-05-31 21:34 UTC (permalink / raw) Cc: emacs-devel >The doc string for delete-process says: > > Delete PROCESS: kill it and forget about it immediately. > >To me 'immediately' means that the value of delete-exited-processes >does not apply here. So I don't agree this is an error. Ok. >> (2) network processes never have their sentinel called a final time > >Right, that's a real bug. > >The following patch should fix that (after undoing your change). > >*** process.c 24 May 2004 09:49:49 +0200 1.429 >--- process.c 31 May 2004 22:28:55 +0200 >*************** >*** 764,769 **** >--- 764,770 ---- > { > XPROCESS (process)->status = Fcons (Qexit, Fcons (make_number (0), Qnil)); > XSETINT (XPROCESS (process)->tick, ++process_tick); >+ status_notify (); > } > else if (XINT (XPROCESS (process)->infd) >= 0) > { > > >Unless you see problems with this, I'll install the patch tomorrow. This was actually the first way I tried to do it, but something bothered me about the fact that the sentinel for a network process can be called while the buffer is still live; that's different than what happens for a buffer with a child process and I was trying to maximize consistency. A program shouldn't have to know too much about how it's communicating with another program, especially if it thinks it's communicating over a network. For example ZenIRC has a hook to use an inferior program to make network connections (e.g. for socks proxying) or to use open-network-stream directly. A sentinel still has to check for open/run or close/exit states, but that's a trivial conditional whereas it's harder to work around the case that sometimes a buffer still exists and sometimes it doesn't. Why should that matter? Well, a sentinel may attempt to reestablish a connection if the remote end disconnected--in that case the buffer will still exist. But if the user kills the buffer, the client should not attempt to reconnect to the remote server. But if killing the buffer invokes the sentinel before the buffer ceases to be active, how can it tell which has happened? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-05-31 21:34 ` Noah Friedman @ 2004-06-02 3:43 ` Richard Stallman 2004-06-02 13:51 ` Kim F. Storm 0 siblings, 1 reply; 16+ messages in thread From: Richard Stallman @ 2004-06-02 3:43 UTC (permalink / raw) Cc: emacs-devel, storm This was actually the first way I tried to do it, but something bothered me about the fact that the sentinel for a network process can be called while the buffer is still live; Do you mean "while the process is still live"? It is normal to call the sentinel for a subprocess while its buffer is still live. But what does this have to do with the buffer, anyway? We could eliminate the difference between network connections and subprocesses in this context by making Fdelete_process call Fkill_process for network connections too. I am not sure whether that change is correct, but it sounds plausible. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-02 3:43 ` Richard Stallman @ 2004-06-02 13:51 ` Kim F. Storm 2004-06-02 22:56 ` Richard Stallman 0 siblings, 1 reply; 16+ messages in thread From: Kim F. Storm @ 2004-06-02 13:51 UTC (permalink / raw) Cc: emacs-devel, Noah Friedman Richard Stallman <rms@gnu.org> writes: > This was actually the first way I tried to do it, but something bothered me > about the fact that the sentinel for a network process can be called while > the buffer is still live; > > Do you mean "while the process is still live"? It is normal to call > the sentinel for a subprocess while its buffer is still live. > But what does this have to do with the buffer, anyway? > > We could eliminate the difference between network connections and > subprocesses in this context by making Fdelete_process call > Fkill_process for network connections too. I am not sure whether that > change is correct, but it sounds plausible. I don't think that will work. You cannot send a signal to a socket, so kill-process does not apply to network processes. IIUC, the problem is that a network process' sentinel would like to be able to differentiate between the case where the connection is closed because the remote end closed the connection, or because it was closed by a call to delete-process. The wish is that the Lisp code does not have to treat using a network process or a subprocess differently. If remote closed the connection (or it was broken for other reasons) we may want to reopen the connection in the sentinel. If the connection was closed via an explicit call to delete-process, I don't think the sentinel should ever try to reopen the connection! Now, Noah says that delete-process may also be called via kill_buffer_processes, but that only happens when we kill the process' buffer, or when we exit emacs -- so in these cases, the sentinel should not try to reconnect either. So IMO, using delete-process should never cause the sentinel to try to reopen the connection. The old code actually did accomplish just that -- but in a brutal way, since the sentinel was not called after delete-process. Now, to let the sentinel treat network processes and subprocesses alike, Noah suggests that the sentinel could look to see if the process' buffer is still alive. But that breaks for network processes. I think looking at the buffer is the wrong approach -- the sentinel must look at the "status" message that it receives (see below). With my proposed change (just call status_notify in Fdelete_process) does indeed allow the sentinel to differentiate between a connection closed via delete-process and by remote (or a broken connection): The string passed to the sentinel is "finished\n" if delete-process was called; otherwise it is "exited abnormally with code 256". Of course, that may be different from what is returned from a subprocess, but the amount of Lisp code needed to treat the extra case is minimal, so what else is needed ? I will undo Noah's change and install my patch shortly. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-02 13:51 ` Kim F. Storm @ 2004-06-02 22:56 ` Richard Stallman 2004-06-02 23:08 ` Noah Friedman 2004-06-03 11:48 ` Kim F. Storm 0 siblings, 2 replies; 16+ messages in thread From: Richard Stallman @ 2004-06-02 22:56 UTC (permalink / raw) Cc: friedman, emacs-devel IIUC, the problem is that a network process' sentinel would like to be able to differentiate between the case where the connection is closed because the remote end closed the connection, or because it was closed by a call to delete-process. If that is the issue, we could arrange for delete-process to call the sentinel in a special way. Perhaps we could have a different state for connections closed by delete-process. Now, to let the sentinel treat network processes and subprocesses alike, Noah suggests that the sentinel could look to see if the process' buffer is still alive. That is clearly the wrong criterion, since after delete-process the buffer would often still be alive. The string passed to the sentinel is "finished\n" if delete-process was called; otherwise it is "exited abnormally with code 256". That general idea is ok, though using the status symbol would be a cleaner way to distinguish. But "finished" is the wrong way to decribe delete-process. "deleted" would be more appropriate. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-02 22:56 ` Richard Stallman @ 2004-06-02 23:08 ` Noah Friedman 2004-06-03 11:48 ` Kim F. Storm 1 sibling, 0 replies; 16+ messages in thread From: Noah Friedman @ 2004-06-02 23:08 UTC (permalink / raw) Cc: emacs-devel, storm > The string passed to the sentinel is "finished\n" if delete-process > was called; otherwise it is "exited abnormally with code 256". > >That general idea is ok, though using the status symbol would >be a cleaner way to distinguish. > >But "finished" is the wrong way to decribe delete-process. >"deleted" would be more appropriate. I'm happy with the idea of using `deleted'. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-02 22:56 ` Richard Stallman 2004-06-02 23:08 ` Noah Friedman @ 2004-06-03 11:48 ` Kim F. Storm 2004-06-03 22:37 ` Kim F. Storm 2004-06-04 2:03 ` Richard Stallman 1 sibling, 2 replies; 16+ messages in thread From: Kim F. Storm @ 2004-06-03 11:48 UTC (permalink / raw) Cc: friedman, emacs-devel Richard Stallman <rms@gnu.org> writes: > IIUC, the problem is that a network process' sentinel would like to > be able to differentiate between the case where the connection is > closed because the remote end closed the connection, or because it > was closed by a call to delete-process. > > If that is the issue, we could arrange for delete-process to call the > sentinel in a special way. Perhaps we could have a different state > for connections closed by delete-process. > > Now, to let the sentinel treat network processes and subprocesses > alike, Noah suggests that the sentinel could look to see if the > process' buffer is still alive. > > That is clearly the wrong criterion, since after delete-process the > buffer would often still be alive. > > The string passed to the sentinel is "finished\n" if delete-process > was called; otherwise it is "exited abnormally with code 256". > > That general idea is ok, though using the status symbol would > be a cleaner way to distinguish. Yes. Here, process-status returns `closed' in both cases, while process-exit-status returns 0 after delete-process and 256 for a broken connection. > > But "finished" is the wrong way to decribe delete-process. > "deleted" would be more appropriate. Normally, you would use delete-process to close the connection. So in that aspect "closed" would be better than "deleted". ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-03 11:48 ` Kim F. Storm @ 2004-06-03 22:37 ` Kim F. Storm 2004-06-03 23:26 ` Noah Friedman 2004-06-04 17:33 ` Richard Stallman 2004-06-04 2:03 ` Richard Stallman 1 sibling, 2 replies; 16+ messages in thread From: Kim F. Storm @ 2004-06-03 22:37 UTC (permalink / raw) Cc: emacs-devel, friedman storm@cua.dk (Kim F. Storm) writes: > Richard Stallman <rms@gnu.org> writes: > > > But "finished" is the wrong way to decribe delete-process. > > "deleted" would be more appropriate. > > Normally, you would use delete-process to close the connection. > > So in that aspect "closed" would be better than "deleted". To continue on this, what is the proper message to pass to the sentinel if the connection was closed by remote peer (or broken). Current message "exited abnormally with code 256" isn't very good, so perhaps we should change that as well (to what ...?) However existing code may rely on the current message, so maybe we need to leave it as is? -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-03 22:37 ` Kim F. Storm @ 2004-06-03 23:26 ` Noah Friedman 2004-06-04 14:20 ` Kim F. Storm 2004-06-04 17:33 ` Richard Stallman 1 sibling, 1 reply; 16+ messages in thread From: Noah Friedman @ 2004-06-03 23:26 UTC (permalink / raw) >To continue on this, what is the proper message to pass to the >sentinel if the connection was closed by remote peer (or broken). > >Current message "exited abnormally with code 256" isn't very good, >so perhaps we should change that as well (to what ...?) > >However existing code may rely on the current message, so maybe we >need to leave it as is? I did a web search just to see if there were any snippets of code which actually tested for this string. There weren't a lot, although it's possible that program sources don't often get indexed. None of the elisp files I have archived, including gnu-emacs-sources archives going back a few years, had any instances of that string literal. ERC was just about the only case I could find and that's one of two applications (the other being zenirc) that prompted me to investigate the sentinel problem in the first place. In fact there hasn't been much reason to examine that string because the process sentinel for network connections has never been called before in this case; only when the remote end closes the connection would it get called. This bug is very old--I can reproduce it in emacs 18.59. >> But "finished" is the wrong way to decribe delete-process. >> "deleted" would be more appropriate. > >Normally, you would use delete-process to close the connection. > >So in that aspect "closed" would be better than "deleted". "closed" seems ambiguous to me; either side could close a connection. But only the local side can delete it. I think changing the process status signal is the most reliable indicator of who requested the disconnection. In the meantime the reason string ought to be made more user-friendly though I don't have any good suggestions. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-03 23:26 ` Noah Friedman @ 2004-06-04 14:20 ` Kim F. Storm 0 siblings, 0 replies; 16+ messages in thread From: Kim F. Storm @ 2004-06-04 14:20 UTC (permalink / raw) Cc: rms, emacs-devel Noah Friedman <friedman@splode.com> writes: > >Normally, you would use delete-process to close the connection. > > > >So in that aspect "closed" would be better than "deleted". > > "closed" seems ambiguous to me; either side could close a connection. Ok, so let's settle on "deleted" and keep "exited abnormally ...". > But only the local side can delete it. I think changing the process status > signal is the most reliable indicator of who requested the disconnection. You can test that with process-exit-status when process-status is closed (by either end). > In the meantime the reason string ought to be made more user-friendly > though I don't have any good suggestions. I think we'll leave it as is, ie. if message is "exited(something)" it was closed by remote. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-03 22:37 ` Kim F. Storm 2004-06-03 23:26 ` Noah Friedman @ 2004-06-04 17:33 ` Richard Stallman 2004-06-06 22:36 ` Kim F. Storm 1 sibling, 1 reply; 16+ messages in thread From: Richard Stallman @ 2004-06-04 17:33 UTC (permalink / raw) Cc: emacs-devel, friedman To continue on this, what is the proper message to pass to the sentinel if the connection was closed by remote peer (or broken). Current message "exited abnormally with code 256" isn't very good, so perhaps we should change that as well (to what ...?) "connection broken by remote peer" looks like a good message to me. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-04 17:33 ` Richard Stallman @ 2004-06-06 22:36 ` Kim F. Storm 0 siblings, 0 replies; 16+ messages in thread From: Kim F. Storm @ 2004-06-06 22:36 UTC (permalink / raw) Cc: emacs-devel, friedman Richard Stallman <rms@gnu.org> writes: > To continue on this, what is the proper message to pass to the > sentinel if the connection was closed by remote peer (or broken). > > Current message "exited abnormally with code 256" isn't very good, > so perhaps we should change that as well (to what ...?) > > "connection broken by remote peer" looks like a good message to me. I installed that change, and changed the local exit message to "deleted" as previously agreed. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-03 11:48 ` Kim F. Storm 2004-06-03 22:37 ` Kim F. Storm @ 2004-06-04 2:03 ` Richard Stallman 2004-06-04 8:10 ` Kim F. Storm 1 sibling, 1 reply; 16+ messages in thread From: Richard Stallman @ 2004-06-04 2:03 UTC (permalink / raw) Cc: emacs-devel, friedman Here, process-status returns `closed' in both cases, while process-exit-status returns 0 after delete-process and 256 for a broken connection. Noah, does that offer your code sufficient info to DTRT? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) 2004-06-04 2:03 ` Richard Stallman @ 2004-06-04 8:10 ` Kim F. Storm 0 siblings, 0 replies; 16+ messages in thread From: Kim F. Storm @ 2004-06-04 8:10 UTC (permalink / raw) Cc: emacs-devel, friedman Richard Stallman <rms@gnu.org> writes: > Here, process-status returns `closed' in both cases, while > process-exit-status returns 0 after delete-process and 256 for > a broken connection. > We should add this information to the manual. -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2004-06-06 22:36 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-05-30 22:34 Last change to process.c breaks fetching pop3 mail (gnus/pop3.el) Kim F. Storm 2004-05-30 23:09 ` Noah Friedman 2004-05-31 21:06 ` Kim F. Storm 2004-05-31 21:34 ` Noah Friedman 2004-06-02 3:43 ` Richard Stallman 2004-06-02 13:51 ` Kim F. Storm 2004-06-02 22:56 ` Richard Stallman 2004-06-02 23:08 ` Noah Friedman 2004-06-03 11:48 ` Kim F. Storm 2004-06-03 22:37 ` Kim F. Storm 2004-06-03 23:26 ` Noah Friedman 2004-06-04 14:20 ` Kim F. Storm 2004-06-04 17:33 ` Richard Stallman 2004-06-06 22:36 ` Kim F. Storm 2004-06-04 2:03 ` Richard Stallman 2004-06-04 8:10 ` Kim F. Storm
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.