GNU bug report logs - #1256
Race condition in vc-diff

Previous Next

Package: emacs;

Reported by: Bob Rogers <rogers-emacs <at> rgrjr.dyndns.org>

Date: Sat, 25 Oct 2008 22:40:03 UTC

Severity: normal

Done: Chong Yidong <cyd <at> stupidchicken.com>

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 1256 in the body.
You can then email your comments to 1256 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-submit-list <at> lists.donarmstrong.com, Emacs Bugs <bug-gnu-emacs <at> gnu.org>:
bug#1256; Package emacs. Full text and rfc822 format available.

Acknowledgement sent to Bob Rogers <rogers-emacs <at> rgrjr.dyndns.org>:
New bug report received and forwarded. Copy sent to Emacs Bugs <bug-gnu-emacs <at> gnu.org>. Full text and rfc822 format available.

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Bob Rogers <rogers-emacs <at> rgrjr.dyndns.org>
To: emacs-pretest-bug <at> gnu.org
Subject: Race condition in vc-diff
Date: Sat, 25 Oct 2008 18:33:01 -0400
[Message part 1 (text/plain, inline)]
   This problem seems to affect only the trunk, and not 22.3.  (I last
built emacs on 21-Oct.)

   To reproduce this, you need a working copy with a VCS that has a fast
backend "diff to base" command; something that doesn't have to contact a
server is probably a requirement (I used SVN with the WC on a local
disk).  You also need a version-controlled file with a trivial change
with respect to the base version; touching a single line is sufficient.

   If you visit this file and do "C-x v =" in a single-window frame that
is more than about 30 lines tall, and if the backend diff command is
fast enough, the diff window will remain half the size of the screen,
rather than being shrunk to fit the buffer.  This is because
vc-exec-after finds that the buffer process has already finished, and
runs vc-diff-finish before the diff buffer has been made visible.  In
that case, vc-diff-finish assumes that the user has already buried it.

   I am not a big fan of this shrink-wrapping, but I do think vc-diff
ought to behave consistently (and it has worked this way for quite a
while now).  The attached patch is sufficient to fix it.

					-- Bob Rogers
					   http://rgrjr.dyndns.org/

[vc-diff-exec-after-1.patch (text/plain, inline)]
--- lisp/vc.el.~1.706.~	2008-10-06 21:02:25.000000000 -0400
+++ lisp/vc.el	2008-10-25 17:49:35.000000000 -0400
@@ -1507,10 +1507,12 @@
       ;; bindings are nicer for read only buffers. pcl-cvs does the
       ;; same thing.
       (setq buffer-read-only t)
-      (vc-exec-after `(vc-diff-finish ,(current-buffer) ',(when verbose
-                                                            messages)))
       ;; Display the buffer, but at the end because it can change point.
       (pop-to-buffer (current-buffer))
+      ;; Do this after pop-to-buffer, because it needs to see the proper window
+      ;; state if it runs immediately.  -- rgr, 25-Oct-08.
+      (vc-exec-after `(vc-diff-finish ,(current-buffer) ',(when verbose
+                                                            messages)))
       ;; In the async case, we return t even if there are no differences
       ;; because we don't know that yet.
       t)))

Information forwarded to bug-submit-list <at> lists.donarmstrong.com, Emacs Bugs <bug-gnu-emacs <at> gnu.org>:
bug#1256; Package emacs. Full text and rfc822 format available.

Acknowledgement sent to Stefan Monnier <monnier <at> iro.umontreal.ca>:
Extra info received and forwarded to list. Copy sent to Emacs Bugs <bug-gnu-emacs <at> gnu.org>. Full text and rfc822 format available.

Message #10 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Bob Rogers <rogers-emacs <at> rgrjr.dyndns.org>
Cc: 1256 <at> debbugs.gnu.org, emacs-pretest-bug <at> gnu.org
Subject: Re: bug#1256: Race condition in vc-diff
Date: Sat, 25 Oct 2008 22:31:18 -0400
>    If you visit this file and do "C-x v =" in a single-window frame that
> is more than about 30 lines tall, and if the backend diff command is
> fast enough, the diff window will remain half the size of the screen,
> rather than being shrunk to fit the buffer.  This is because
> vc-exec-after finds that the buffer process has already finished, and
> runs vc-diff-finish before the diff buffer has been made visible.  In
> that case, vc-diff-finish assumes that the user has already buried it.

>    I am not a big fan of this shrink-wrapping, but I do think vc-diff
> ought to behave consistently (and it has worked this way for quite a
> while now).  The attached patch is sufficient to fix it.

As the comment indicates, this patch is not quite good enough because
pop-to-buffer may change point.  We probably will need to manipulate
point explicitly to work around the problem.  BTW you can make the
"race-condition" deterministic by making the diff command synchronous
(as is the case with the RCS backend, for example, where your problem
should just *always* happen).


        Stefan




Information forwarded to bug-submit-list <at> lists.donarmstrong.com, Emacs Bugs <bug-gnu-emacs <at> gnu.org>:
bug#1256; Package emacs. Full text and rfc822 format available.

Acknowledgement sent to Stefan Monnier <monnier <at> iro.umontreal.ca>:
Extra info received and forwarded to list. Copy sent to Emacs Bugs <bug-gnu-emacs <at> gnu.org>. Full text and rfc822 format available.

Information forwarded to bug-submit-list <at> lists.donarmstrong.com, Emacs Bugs <bug-gnu-emacs <at> gnu.org>:
bug#1256; Package emacs. Full text and rfc822 format available.

Acknowledgement sent to Bob Rogers <rogers-emacs <at> rgrjr.dyndns.org>:
Extra info received and forwarded to list. Copy sent to Emacs Bugs <bug-gnu-emacs <at> gnu.org>. Full text and rfc822 format available.

Message #20 received at 1256 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Bob Rogers <rogers-emacs <at> rgrjr.dyndns.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 1256 <at> debbugs.gnu.org
Subject: Re: bug#1256: Race condition in vc-diff
Date: Sat, 25 Oct 2008 23:45:39 -0400
   From: Stefan Monnier <monnier <at> iro.umontreal.ca>
   Date: Sat, 25 Oct 2008 22:31:18 -0400

   >    If you visit this file and do "C-x v =" in a single-window frame that
   > is more than about 30 lines tall, and if the backend diff command is
   > fast enough, the diff window will remain half the size of the screen,
   > rather than being shrunk to fit the buffer.  This is because
   > vc-exec-after finds that the buffer process has already finished, and
   > runs vc-diff-finish before the diff buffer has been made visible.  In
   > that case, vc-diff-finish assumes that the user has already buried it.

   >    I am not a big fan of this shrink-wrapping, but I do think vc-diff
   > ought to behave consistently (and it has worked this way for quite a
   > while now).  The attached patch is sufficient to fix it.

   As the comment indicates, this patch is not quite good enough because
   pop-to-buffer may change point.  We probably will need to manipulate
   point explicitly to work around the problem.  

Problem?  vc-diff-finish sets point in the diff buffer, so if it runs
immediately, it seems to me that it ought to override what pop-to-buffer
does.  But perhaps I don't understand what point-changing behavior you
mean.

   BTW you can make the "race-condition" deterministic by making the
   diff command synchronous (as is the case with the RCS backend, for
   example, where your problem should just *always* happen).

	   Stefan

It never occurred to me to try that.  However, the same argument would
seem to apply for (setq vc-disable-async-diff t), but that only makes
the bug appear intermittently for me, even with the identical test case.

					-- Bob




bug reassigned from package `emacs' to `emacs,vc'. Request was from Juanma Barranquero <lekktu <at> gmail.com> to control <at> emacsbugs.donarmstrong.com. (Tue, 17 Mar 2009 09:30:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-submit-list <at> lists.donarmstrong.com, Emacs Bugs <bug-gnu-emacs <at> gnu.org>, owner <at> emacsbugs.donarmstrong.com:
bug#1256; Package emacs,vc. (Thu, 30 Apr 2009 18:30:03 GMT) Full text and rfc822 format available.

View this message in rfc822 format

From: Chong Yidong <cyd <at> stupidchicken.com>
To: Bob Rogers <rogers-emacs <at> rgrjr.dyndns.org>
Cc: 1256 <at> debbugs.gnu.org
Subject: bug#1256: Race condition in vc-diff
Date: Sat, 09 Jul 2011 21:44:02 -0400
>    If you visit this file and do "C-x v =" in a single-window frame
> that is more than about 30 lines tall, and if the backend diff command
> is fast enough, the diff window will remain half the size of the
> screen, rather than being shrunk to fit the buffer.  This is because
> vc-exec-after finds that the buffer process has already finished, and
> runs vc-diff-finish before the diff buffer has been made visible.  In
> that case, vc-diff-finish assumes that the user has already buried it.
>
>    I am not a big fan of this shrink-wrapping, but I do think vc-diff
> ought to behave consistently (and it has worked this way for quite a
> while now).  The attached patch is sufficient to fix it.

I committed the patch to trunk.  Thanks.




bug closed, send any further explanations to 1256 <at> debbugs.gnu.org and Bob Rogers <rogers-emacs <at> rgrjr.dyndns.org> Request was from Chong Yidong <cyd <at> stupidchicken.com> to control <at> debbugs.gnu.org. (Sun, 10 Jul 2011 01:45:05 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 07 Aug 2011 11:24:13 GMT) Full text and rfc822 format available.

This bug report was last modified 13 years and 317 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.