[ patch ] async stuff fix(?) (was Re: 0.3 and async pre-fetch)

[Sudish Joseph <sudish@mindspring.com>]

In article <m2lofxi9tl.fsf@atreides.erehwon.org>,
Sudish Joseph <sudish@mindspring.com> writes:
> nntp-process-filter should *never* use re-search-backward to identify
> the end of a command's output.

Here's a patch that uses forward search over the string received by
the filter.  My limited testing (3 groups of 50-100 articles) shows
things as being ok, but I think this could do with a rewrite, more
below. 

Fixing this exposed another bug: gnus-async-prefetch-article could get
confused about which part of the async buffer was the actual article.
This was really painful, coz it was unstable (one munged article meant
that a lot of following articles would be munged).  After reading
about 100 articles of a 1,500 article news.groups summary, it
completely lost track of which article was which.

The problem here was that there were two different functions trying to
define "this is the article" independent of the other -- the filter
and gnus-async-prefetch-article.  I've rewritten the callback to have
the filter pass it the required markers so that it no longer tries to
assume anything about which part of the async buffer is the article.
No problem, so far...even if one shows up, this should be a stable
error as subsequent articles won't get munged.

The rewrite I mentioned: 

1) A cleaner interface between gnus-async and nntp would have
gnus-async sending requests to nntp and nntp passing back each
response as a string; i.e., they will each manipulate their own
buffers and markers, with *no* exceptions...the current interface has
them doing each others laundry.  What I've done is a weak compromise.

The downside to using strings is that we'll get -large- strings that
will hog memory until garbage collection.  Is there any way to
truncate the actual string to get around this?  (It has to be there,
I'm too lazy to dig right now.)  Even better, any way to totally
obliterate the in-memory string; i.e., gc a single string?  I don't
think the number of actual strings generated will increase gc costs.
If so, we might keep a constant number of strings that we grow and
truncate as needed.  Hmm, do buffers cost more/less than strings?  We
might pass buffers as our result from the filter if that's better.
Buffers aren't gc'ed I think, but we can manually maintain those quite
easily.

2) Eliminate the need to generate callbacks dynamically in
gnus-async-prefetch-article by having that function keep a queue of
sent commands that'll be used by a static callback (a defun'ed,
non-backquote generated, function) to match group/article combinations
to received responses (assuming that responses are received in the
correct order...seems reasonable).  Keep that queue buffer-local to
allow for multiple summary prefetches to work at once.

This would a) make debugging this stuff stop being a major pita, 

b) make it easier to understand, c) reduce gc (there's a lambda
discarded for every article prefetched currently), d) make it easier
to guarantee that responses are tagged with the correct group-article
id.

a + b are good enough for me, though c + d are the reasons that might
influence you. :)

There'll definitely be more wrinkles in this stuff, rewriting it to be
simpler to debug/understand is worth doing right now.

The above doesn't involve much coding, though my usual bombasticity
might make it seem so.

-Sudish

PS:  If anyone would care to explain: can buffers be gc'ed?  are
lambda's gc'ed (if not, gnus currently leaks)?  can an arbitrary
object be completely destroyed/gc'ed all by itself?

Sun Aug  4 17:31:53 1996  Sudish Joseph  <sudish@mindspring.com>

	* nntp.el (nntp-tmp-first): Unused variable, deleted.
	(nntp-process-filter): Determine end-of-response using a forward
	search over STRING.  Don't assume that this in the only response in
	the process buffer.  Generate markers for the beginning and end
	of the response inserted into the async prefetch buffer.

	* gnus-async.el (gnus-async-prefetch-article): The generated 
	callback now assumes that the process filter will pass it the
	required markers as (optional) arguments.


*** rgnus-0.05/lisp/gnus-async.el	Fri Aug  2 14:04:55 1996
--- lisp/gnus-async.el	Sun Aug  4 17:14:42 1996
***************
*** 94,112 ****
        ;; We want to fetch some more articles.
        (save-excursion
  	(set-buffer summary)
! 	(let ((next (caadr (gnus-data-find-list article)))
! 	      mark)
  	  (nnheader-set-temp-buffer gnus-async-prefetch-article-buffer t)
- 	  (goto-char (point-max))
- 	  (setq mark (point-marker))
  	  (let ((nnheader-callback-function
! 		 `(lambda (arg)
  		    (save-excursion
  		      (nnheader-set-temp-buffer
  		       gnus-async-prefetch-article-buffer t)
  		      (push (list ',(intern (format "%s-%d" group article))
! 				  ,mark (set-marker (make-marker) (point-max))
! 				  ,group ,article)
  			    gnus-async-article-alist)
  		      (when (gnus-buffer-live-p ,summary)
  			,(when next
--- 94,108 ----
        ;; We want to fetch some more articles.
        (save-excursion
  	(set-buffer summary)
! 	(let ((next (caadr (gnus-data-find-list article))))
  	  (nnheader-set-temp-buffer gnus-async-prefetch-article-buffer t)
  	  (let ((nnheader-callback-function
! 		 `(lambda (arg &optional begin-marker end-marker)
  		    (save-excursion
  		      (nnheader-set-temp-buffer
  		       gnus-async-prefetch-article-buffer t)
  		      (push (list ',(intern (format "%s-%d" group article))
! 				  begin-marker end-marker ,group ,article)
  			    gnus-async-article-alist)
  		      (when (gnus-buffer-live-p ,summary)
  			,(when next
*** rgnus-0.05/lisp/nntp.el	Sat Aug  3 18:50:09 1996
--- lisp/nntp.el	Sun Aug  4 17:14:42 1996
***************
*** 476,482 ****
  	    (eval (cadr entry))
  	  (funcall (cadr entry)))))))
  
- (defvar nntp-tmp-first)
  (defvar nntp-tmp-wait-for)
  (defvar nntp-tmp-callback)
  (defvar nntp-tmp-buffer)
--- 476,481 ----
***************
*** 492,500 ****
    "Process filter used for waiting a calling back."
    (let ((old-buffer (current-buffer)))
      (unwind-protect
! 	(let (point)
  	  (set-buffer (process-buffer proc))
! 	  ;; Insert the text, moving the process-marker.
  	  (setq point (goto-char (process-mark proc)))
  	  (insert string)
  	  (set-marker (process-mark proc) (point))
--- 491,499 ----
    "Process filter used for waiting a calling back."
    (let ((old-buffer (current-buffer)))
      (unwind-protect
! 	(let (point eoresponse b e)
  	  (set-buffer (process-buffer proc))
! 	  ;; Insert string, moving the process-marker.
  	  (setq point (goto-char (process-mark proc)))
  	  (insert string)
  	  (set-marker (process-mark proc) (point))
***************
*** 504,520 ****
  		(nntp-snarf-error-message)
  		(set-process-filter proc nil)
  		(funcall nntp-tmp-callback nil))
! 	    (setq nntp-tmp-first nil)
! 	    (if (re-search-backward nntp-tmp-wait-for nil t)
  		(progn
  		  (if (buffer-name (get-buffer nntp-tmp-buffer))
  		      (save-excursion
  			(set-buffer (get-buffer nntp-tmp-buffer))
  			(goto-char (point-max))
! 			(insert-buffer-substring (process-buffer proc))))
! 		  (set-process-filter proc nil)
! 		  (erase-buffer)
! 		  (funcall nntp-tmp-callback t)))))
        (set-buffer old-buffer))))
  
  (defun nntp-retrieve-data (command address port buffer
--- 503,541 ----
  		(nntp-snarf-error-message)
  		(set-process-filter proc nil)
  		(funcall nntp-tmp-callback nil))
! 	    ;; Check if the inserted string contained the end of a response.
! 	    (goto-char point)
! 	    (if (re-search-forward nntp-tmp-wait-for nil t)
! 		;; we have a complete response, copy it into the specified
! 		;; buffer
  		(progn
+ 		  (setq eoresponse (point))
  		  (if (buffer-name (get-buffer nntp-tmp-buffer))
  		      (save-excursion
  			(set-buffer (get-buffer nntp-tmp-buffer))
+ 			;; we insert at the end; this is safe coz we're
+ 			;; the only one inserting into this buffer
  			(goto-char (point-max))
! 			;; We have what we know to be a full
! 			;; response. Since we're the ones inserting
! 			;; it, we know exactly where it began and
! 			;; ended.  We pass this information to the
! 			;; callback, eliminating the need for
! 			;; the callback to guess this for itself and
! 			;; eliminating unnecessary complexity there.
! 			;; This also has the advantage of making prefetch
! 			;; self-stabilizing in the face of errors -- i.e.,
! 			;; if an error occurs, it'll only affect one article
! 			;; and subsequent articles won't be munged. -- sj
! 			(setq b (point-marker))
! 			(insert-buffer-substring (process-buffer proc)
! 						 1 eoresponse)
! 			(setq e (point-marker))))
! 		  ;; delete the processed response
! 		  (delete-region 1 eoresponse)
! 		  ;; notify the async system that it has a full response to
! 		  ;; process, passing it the beginning and end of that resp.
! 		  (funcall nntp-tmp-callback t b e)))))
        (set-buffer old-buffer))))
  
  (defun nntp-retrieve-data (command address port buffer