Problems with latest CVS.

Problems with latest CVS.

[Luc Teirlinck]

Does anybody else see any of this?

With the very latest CVS Emacs used to crash rather quickly after
startup after typing a bunch of keystrokes.  After reverting this
change:

   2004-02-16  Dmitry Antipov  <dmitry.antipov@mail.ru>  (tiny change)

	    * keyboard.c (prev_read): New static variable.
	    (read_avail_input): Use it to zero out only those slots in
	    buf[]
	    that were used last time we were called.

Emacs no longer seems to crash.

However, even after this revert old minibuffer messages about reading
directories keep getting redisplayed over and over again in rapid
succession, even after a subsequent quit.  An idle Emacs chews up
nearly twenty percent of CPU.

I do seem to have problems with my disk, but I do not believe that can
be the cause.  I just marked all my bad blocks bad and repaired my
file system.  Still, does anybody else see this with the very latest
CVS (less than 3 hours old)?

(emacs-version):
"GNU Emacs 21.3.50.1 (i686-pc-linux-gnu, X toolkit)\n of 2004-02-16 on swt40.swt.com"

Sincerely,

Luc.

Re: Problems with latest CVS.

[Luc Teirlinck]

I forgot to mention that the way to reproduce the problem with
constant messaging, even after a quit, is to use C-x d a couple of
times on different directories.

Sincerely,

Luc.

Re: Problems with latest CVS.

[Luc Teirlinck]

The messaging problem is due to one of my customizations.  It does not
occur with `emacs -q'.  (I should have checked this earlier, sorry.)
I will file a proper report once I have found out the details.

Sincerely,

Luc.

Re: Problems with latest CVS.

[Luc Teirlinck]

I believe I figured the problem out.  Continuous noisy and
CPU-consuming reverting of dired buffers occurs, _regardless_ of the
value of `global-auto-revert-non-file-buffers', if
global-autorevert-mode is enabled and the user either does not have
the "a" switch in dired-listing-switches or if the user inserts
subdirectories.

There are two problems:

1.  `global-auto-revert-non-file-buffers' was ignored for dired
    buffers.  I already committed a fix to CVS for that one, because
    that one seemed to be the urgent one:
    `global-auto-revert-non-file-buffers' is a hyper-new option whose
    default value is nil.

2.  To determine whether a dired buffer has changed,
    `auto-revert-dired-changed-p' just counts the number of files in
    the dired buffer and compares it with the number of files listed
    by (directory-files dired-directory).  This is completely
    unreliable because of dired-listing-switches, because of inserted
    subdirectories, and probably because of other reasons as well.

I did not fix the second problem.

Sincerely,

Luc.

Re: Problems with latest CVS.

[Luc Teirlinck]

>From my previous message:

    Continuous noisy and CPU-consuming reverting of dired buffers
   occurs, _regardless_ of the value of
   `global-auto-revert-non-file-buffers', if global-autorevert-mode is
   enabled and the user either does not have the "a" switch in
   dired-listing-switches or if the user inserts subdirectories.

To clarify: after my fix of one of the two problems the described
problem no longer occurs if `global-auto-revert-non-file-buffers' is
nil.  It still occurs if it is non-nil.

Sincerely,

Luc.

Re: Problems with latest CVS.

[Eli Zaretskii]

> Date: Mon, 16 Feb 2004 20:49:18 -0600 (CST)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> 
> 1.  `global-auto-revert-non-file-buffers' was ignored for dired
>     buffers.  I already committed a fix to CVS for that one, because
>     that one seemed to be the urgent one:
>     `global-auto-revert-non-file-buffers' is a hyper-new option whose
>     default value is nil.
> 
> 2.  To determine whether a dired buffer has changed,
>     `auto-revert-dired-changed-p' just counts the number of files in
>     the dired buffer and compares it with the number of files listed
>     by (directory-files dired-directory).  This is completely
>     unreliable because of dired-listing-switches, because of inserted
>     subdirectories, and probably because of other reasons as well.
> 
> I did not fix the second problem.

At Richard's request, I installed yesterday several changes in
autorevert.el, perhaps some of them cause these problems.

Re: Problems with latest CVS.

[Luc Teirlinck]

Eli Zaretskii wrote:

   At Richard's request, I installed yesterday several changes in
   autorevert.el, perhaps some of them cause these problems.

Yes they do cause the problems.  I installed a fix for the more urgent
part of the problem yesterday.  The patch below seems to fix the
second part of the problem.  Because the remaining problem only occurs
if `global-auto-revert-non-file-buffers' is non-nil and the default is
nil, fixing the remaining part of the problem is less urgent.  Hence, I
will let people take a look at it before installing.  I can install if
desired.  

Note that part of the patch, namely checking (buffer-modified-p), is,
in addition to the reasons already mentioned, also necessary to allow
auto-reverting dired buffers to co-exist with commands such as
dired-undo and the like.

In my own testing, the patch seems to work well and no significant
CPU-usage seems to occur, even after visiting tons of dired buffers.
My dired-listing-switches's value is "-AltF" and without the patch,
Emacs becomes essentially unusable (and chews up tons of CPU) if
`global-auto-revert-non-file-buffers' is set to t.

===File ~/autorevert-diff===================================
*** autorevert.el.~1.21.~	Mon Feb 16 20:04:45 2004
--- autorevert.el	Tue Feb 17 09:53:15 2004
***************
*** 283,312 ****
  	(forward-line 1)))
      list))
  
- (defun auto-revert-dired-changed-p ()
-   "Check if dired buffer has changed."
-   (when (and (stringp dired-directory)
- 	     ;;	  Exclude remote buffers, would be too slow for user
- 	     ;;	  modem, timeouts, network lag ... all is possible
- 	     (not (string-match "@" dired-directory))
- 	     (file-directory-p dired-directory))
-     (let ((files (directory-files dired-directory))
- 	  (dired (auto-revert-dired-file-list)))
-       (or (not (eq (length files) (length dired)))
- 	  (auto-revert-list-diff files dired)))))
- 
  (defun auto-revert-buffer-p ()
    "Check if current buffer should be reverted."
!   ;;  - Always include dired buffers to list.  It would be too expensive
    ;;  to test the "revert" status here each time timer launches.
-   ;;  - Same for VC buffers.
    (or (and (eq major-mode 'dired-mode)
  	   (or (and global-auto-revert-mode
  		    global-auto-revert-non-file-buffers)
! 	       auto-revert-mode))
!       (and (not (buffer-modified-p))
  	   (auto-revert-vc-buffer-p))
!       (and (not (buffer-modified-p))
  	   (if (buffer-file-name)
  	       (and (file-readable-p (buffer-file-name))
  		    (not (verify-visited-file-modtime (current-buffer))))
--- 283,314 ----
  	(forward-line 1)))
      list))
  
  (defun auto-revert-buffer-p ()
    "Check if current buffer should be reverted."
!   ;;  - Always include VC buffers to list.  It would be too expensive
    ;;  to test the "revert" status here each time timer launches.
    (or (and (eq major-mode 'dired-mode)
  	   (or (and global-auto-revert-mode
  		    global-auto-revert-non-file-buffers)
! 	       auto-revert-mode)
! 	   (not (buffer-modified-p))
! 	   (stringp dired-directory)
! 	   ;; Exclude remote buffers, would be too slow for user
! 	   ;; modem, timeouts, network lag ... all is possible
! 	   (not (string-match "@" dired-directory))
! 	   (file-directory-p dired-directory)
!            (file-readable-p dired-directory)
! 	   (not (let ((attributes (file-attributes dired-directory))
! 		      (modtime (visited-file-modtime)))
! 		  (or (eq modtime 0)
! 		      (not (eq (car attributes) t))
! 		      (and (= (car (nth 5 attributes)) (car modtime))
! 			   (= (nth 1 (nth 5 attributes)) (cdr modtime)))))))
!       (and (not (eq major-mode 'dired-mode))
! 	   (not (buffer-modified-p))
  	   (auto-revert-vc-buffer-p))
!       (and (not (eq major-mode 'dired-mode))
! 	   (not (buffer-modified-p))
  	   (if (buffer-file-name)
  	       (and (file-readable-p (buffer-file-name))
  		    (not (verify-visited-file-modtime (current-buffer))))
***************
*** 375,382 ****
      (cond
       ((eq major-mode 'dired-mode)
        ;;  Dired includes revert-buffer-function
!       (when (and revert-buffer-function
! 		 (auto-revert-dired-changed-p))
  	(setq revert t)))
       ((auto-revert-vc-buffer-p)
        (when (auto-revert-handler-vc)
--- 377,383 ----
      (cond
       ((eq major-mode 'dired-mode)
        ;;  Dired includes revert-buffer-function
!       (when revert-buffer-function
  	(setq revert t)))
       ((auto-revert-vc-buffer-p)
        (when (auto-revert-handler-vc)
============================================================

Re: Problems with latest CVS.

[Luc Teirlinck]

>From my patch:

   (let ((attributes (file-attributes dired-directory))
   ! 		      (modtime (visited-file-modtime)))
   ! 		  (or (eq modtime 0)
   ! 		      (not (eq (car attributes) t))
   ! 		      (and (= (car (nth 5 attributes)) (car modtime))
   ! 			   (= (nth 1 (nth 5 attributes)) (cdr modtime)))))

I forgot to mention that this part of my patch is, up to minor
adaptation, the same code that dired itself uses to check whether the
dired buffer needs updating.

Sincerely,

Luc.

Re: Problems with latest CVS.

[Luc Teirlinck]

If we go for my patch, we could also delete `auto-revert-list-diff'
and `auto-revert-dired-file-list', which are no longer used.

It should be noted that neither the original version nor my patch
automatically update a dired buffer in _all_ situations where updating
might yield _some_ new information.  My patch updates automatically in
exactly the same situations where dired warns that the directory needs
updating after one revisits an obsolete dired buffer, except that it
does no automatic updating if the dired buffer is modified (not to
derail dired-undo and the like).

Sincerely,

Luc.

Re: Problems with latest CVS.

[Luc Teirlinck]

>From my previous message:

   It should be noted that neither the original version nor my patch
   automatically update a dired buffer in _all_ situations where updating
   might yield _some_ new information.

For instance, the patch does not automatically update for changes in
modification times for listed subdirectories, although it does update
for changes in modification times for listed _files_.  It does not seem
that the original updates for either.  In general, the patch seems to
update for a lot more information than the original.

Sincerely,

Luc.

Re: Problems with latest CVS.

[Luc Teirlinck]

Latest patch.  (Deletes two more functions and adds to a docstring.
No other changes.)  I could commit if desired.

===File ~/autorevert-newdiff================================
*** autorevert.el.~1.21.~	Mon Feb 16 20:04:45 2004
--- autorevert.el	Tue Feb 17 18:25:15 2004
***************
*** 158,164 ****
  When non-nil, both file buffers and buffers with a custom
  `revert-buffer-function' are reverted by Global Auto-Revert Mode.
  
! Use this option with care since it could lead to excessive reverts."
    :group 'auto-revert
    :type 'boolean)
  
--- 158,169 ----
  When non-nil, both file buffers and buffers with a custom
  `revert-buffer-function' are reverted by Global Auto-Revert Mode.
  
! Use this option with care since it could lead to excessive reverts.
! Note also that for some non-file buffers the check whether the
! buffer needs updating may be imperfect, due to efficiency
! considerations, and may not take all information listed in the
! buffer into account.  Hence, a non-nil value for this option does
! not necessarily make manual updates useless for non-file buffers."
    :group 'auto-revert
    :type 'boolean)
  
***************
*** 260,312 ****
         (not (memq major-mode
  		  global-auto-revert-ignore-modes)))))
  
- (defun auto-revert-list-diff (a b)
-   "Check if strings in list A differ from list B."
-   (when (and a b)
-     (setq a (sort a 'string-lessp))
-     (setq b (sort b 'string-lessp))
-     (let (elt1 elt2)
-       (catch 'break
- 	(while (and (setq elt1 (and a (pop a)))
- 		    (setq elt2 (and b (pop b))))
- 	  (if (not (string= elt1 elt2))
- 	      (throw 'break t)))))))
- 
- (defun auto-revert-dired-file-list ()
-   "Return list of dired files."
-   (let (file list)
-     (save-excursion
-       (goto-char (point-min))
-       (while (not (eobp))
- 	(if (setq file (dired-get-filename t t))
- 	    (push file list))
- 	(forward-line 1)))
-     list))
- 
- (defun auto-revert-dired-changed-p ()
-   "Check if dired buffer has changed."
-   (when (and (stringp dired-directory)
- 	     ;;	  Exclude remote buffers, would be too slow for user
- 	     ;;	  modem, timeouts, network lag ... all is possible
- 	     (not (string-match "@" dired-directory))
- 	     (file-directory-p dired-directory))
-     (let ((files (directory-files dired-directory))
- 	  (dired (auto-revert-dired-file-list)))
-       (or (not (eq (length files) (length dired)))
- 	  (auto-revert-list-diff files dired)))))
- 
  (defun auto-revert-buffer-p ()
    "Check if current buffer should be reverted."
!   ;;  - Always include dired buffers to list.  It would be too expensive
    ;;  to test the "revert" status here each time timer launches.
-   ;;  - Same for VC buffers.
    (or (and (eq major-mode 'dired-mode)
  	   (or (and global-auto-revert-mode
  		    global-auto-revert-non-file-buffers)
! 	       auto-revert-mode))
!       (and (not (buffer-modified-p))
  	   (auto-revert-vc-buffer-p))
!       (and (not (buffer-modified-p))
  	   (if (buffer-file-name)
  	       (and (file-readable-p (buffer-file-name))
  		    (not (verify-visited-file-modtime (current-buffer))))
--- 265,296 ----
         (not (memq major-mode
  		  global-auto-revert-ignore-modes)))))
  
  (defun auto-revert-buffer-p ()
    "Check if current buffer should be reverted."
!   ;;  - Always include VC buffers to list.  It would be too expensive
    ;;  to test the "revert" status here each time timer launches.
    (or (and (eq major-mode 'dired-mode)
  	   (or (and global-auto-revert-mode
  		    global-auto-revert-non-file-buffers)
! 	       auto-revert-mode)
! 	   (not (buffer-modified-p))
! 	   (stringp dired-directory)
! 	   ;; Exclude remote buffers, would be too slow for user
! 	   ;; modem, timeouts, network lag ... all is possible
! 	   (not (string-match "@" dired-directory))
! 	   (file-directory-p dired-directory)
!            (file-readable-p dired-directory)
! 	   (not (let ((attributes (file-attributes dired-directory))
! 		      (modtime (visited-file-modtime)))
! 		  (or (eq modtime 0)
! 		      (not (eq (car attributes) t))
! 		      (and (= (car (nth 5 attributes)) (car modtime))
! 			   (= (nth 1 (nth 5 attributes)) (cdr modtime)))))))
!       (and (not (eq major-mode 'dired-mode))
! 	   (not (buffer-modified-p))
  	   (auto-revert-vc-buffer-p))
!       (and (not (eq major-mode 'dired-mode))
! 	   (not (buffer-modified-p))
  	   (if (buffer-file-name)
  	       (and (file-readable-p (buffer-file-name))
  		    (not (verify-visited-file-modtime (current-buffer))))
***************
*** 375,382 ****
      (cond
       ((eq major-mode 'dired-mode)
        ;;  Dired includes revert-buffer-function
!       (when (and revert-buffer-function
! 		 (auto-revert-dired-changed-p))
  	(setq revert t)))
       ((auto-revert-vc-buffer-p)
        (when (auto-revert-handler-vc)
--- 359,365 ----
      (cond
       ((eq major-mode 'dired-mode)
        ;;  Dired includes revert-buffer-function
!       (when revert-buffer-function
  	(setq revert t)))
       ((auto-revert-vc-buffer-p)
        (when (auto-revert-handler-vc)
============================================================

Re: Problems with latest CVS.

[Kim F. Storm]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Does anybody else see any of this?
> 
> With the very latest CVS Emacs used to crash rather quickly after
> startup after typing a bunch of keystrokes.  After reverting this
> change:
> 
>    2004-02-16  Dmitry Antipov  <dmitry.antipov@mail.ru>  (tiny change)
> 
> 	    * keyboard.c (prev_read): New static variable.
> 	    (read_avail_input): Use it to zero out only those slots in
> 	    buf[]
> 	    that were used last time we were called.
> 
> Emacs no longer seems to crash.

It was using uninitialized data on the stack.

I have installed a (hopefully) correct fix for this, preserving the
intention of only clearing what's being used.

BTW, I also installed another fix a hour ago or so, but it wasn't ok,
as it caused emacs to loop in GC.  My latest fix (properly handling
re-entrance to read_avail_input) seems to work alright.

If there are still problems, I suggest to revert keyboard.c back
to revision 1.761.

Re: Problems with latest CVS.

[Luc Teirlinck]

Kim Storm wrote:

   My latest fix (properly handling re-entrance to read_avail_input)
   seems to work alright.

It indeed seems to.  Thanks.

Sincerely,

Luc.