bug#29348: 26.0.50; shr.el heisenbug

bug#29348: 26.0.50; shr.el heisenbug

[Devon Sean McCullough]

Given the same input `shr-render-buffer' and `shr-render-region'
scramble their output about 25% of the time.  Here's a fix:
diff -Bbdu old/shr.el new/shr.el
--- old/shr.el	2017-06-06 16:19:10.000000000 -0400
+++ new/shr.el	2017-11-18 01:35:01.000000000 -0500
@@ -198,6 +198,30 @@
       (set-keymap-parent map image-map))
     map))
 
+(eval-when (compile eval)
+  (defmacro shr-with-temp-window (&rest body)
+    "Temporarily give the current buffer some window and evaluate BODY there like progn."
+    ;; Avoid the `save-window-excursion' heisenbug
+    ;; which sporadically clobbers some other buffer's point
+    ;; jumbling `shr-render-buffer' and `shr-render-region' output.
+    (let ((c (gensym "WCONFIG"))
+	  (p (gensym "POINT")))
+      `(let* ((,c (current-window-configuration))
+	      (,p (with-current-buffer (window-buffer (selected-window))
+		    (point)))
+	      ;; Inhibit any `set-window-buffer' hooks:
+	      window-scroll-functions
+	      window-configuration-change-hook)
+	 (unwind-protect
+	     (progn
+	       ;; Avoid errors if the selected window is strongly dedicated.
+	       (set-window-dedicated-p nil nil)
+	       (set-window-buffer nil (current-buffer))
+	       .,body)
+	   (set-window-configuration ,c)
+	   (with-current-buffer (window-buffer (selected-window))
+	     (set-window-point (selected-window) (goto-char ,p))))))))
+
 ;; Public functions and commands.
 (declare-function libxml-parse-html-region "xml.c"
 		  (start end &optional base-url discard-comments))
@@ -539,14 +563,10 @@
 (defun shr-pixel-column ()
   (if (not shr-use-fonts)
       (current-column)
-    (if (not (get-buffer-window (current-buffer)))
-	(save-window-excursion
-          ;; Avoid errors if the selected window is a dedicated one,
-          ;; and they just want to insert a document into it.
-          (set-window-dedicated-p nil nil)
-	  (set-window-buffer nil (current-buffer))
-	  (car (window-text-pixel-size nil (line-beginning-position) (point))))
-      (car (window-text-pixel-size nil (line-beginning-position) (point))))))
+    (if (get-buffer-window)
+	(car (window-text-pixel-size nil (line-beginning-position) (point)))
+      (shr-with-temp-window
+	(car (window-text-pixel-size nil (line-beginning-position) (point)))))))
 
 (defun shr-pixel-region ()
   (- (shr-pixel-column)
@@ -2228,13 +2248,9 @@
 	    (forward-line 1))
 	  max))
     (if (get-buffer-window)
-	(car (window-text-pixel-size nil (point-min) (point-max)))
-      (save-window-excursion
-        ;; Avoid errors if the selected window is a dedicated one,
-        ;; and they just want to insert a document into it.
-        (set-window-dedicated-p nil nil)
-	(set-window-buffer nil (current-buffer))
-	(car (window-text-pixel-size nil (point-min) (point-max)))))))
+	(car (window-text-pixel-size))
+      (shr-with-temp-window
+	(car (window-text-pixel-size))))))
 
 (defun shr-render-td (dom width fill)
   (let ((cache (intern (format "shr-td-cache-%s-%s" width fill))))
@@ -2275,11 +2291,7 @@
       (let ((shr-internal-width width)
 	    (shr-indentation 0))
 	(shr-descend dom))
-      (save-window-excursion
-        ;; Avoid errors if the selected window is a dedicated one,
-        ;; and they just want to insert a document into it.
-        (set-window-dedicated-p nil nil)
-	(set-window-buffer nil (current-buffer))
+      (shr-with-temp-window
 	(unless fill
 	  (setq natural-width
 		(or (dom-attr dom 'shr-td-cache-natural)

In GNU Emacs 26.0.50 (build 1, x86_64-apple-darwin13.4.0, NS appkit-1265.21 Version 10.9.5 (Build 13F1911))
 of 2017-06-07 built on builder10-9.local
Windowing system distributor 'Apple', version 10.3.1404
Recent messages:
Warning: arch-dependent data dir ’/Users/build/workspace/Emacs-Multi-Build/label/mavericks/emacs-source/nextstep/Emacs.app/Contents/MacOS/libexec/’: No such file or directory

For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp' --with-modules'

Configured features:
NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message subr-x puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/ns-win
ns-win ucs-normalize mule-util term/common-win tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow
isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote kqueue cocoa ns multi-tty
make-network-process emacs)

Memory information:
((conses 16 203281 5816)
 (symbols 48 19810 1)
 (miscs 40 43 125)
 (strings 32 17913 6360)
 (string-bytes 1 581057)
 (vectors 16 34820)
 (vector-slots 8 694299 11533)
 (floats 8 48 182)
 (intervals 56 204 0)
 (buffers 976 11))

bug#29348: 26.0.50; shr.el heisenbug

[Eli Zaretskii]

> From: Devon Sean McCullough <Emacs-Hacker2017@jovi.net>
> Date: Sat, 18 Nov 2017 02:19:29 -0500
> 
> Given the same input `shr-render-buffer' and `shr-render-region'
> scramble their output about 25% of the time.  Here's a fix:

Can you tell more about this "scrambling"?  I don't understand what
that means.  An example where that happens would also be useful.

Thanks.

bug#29348: 26.0.50; shr.el heisenbug

[Eli Zaretskii]

> From: Devon Sean McCullough <Emacs-Hacker2017@jovi.net>
> Date: Sat, 18 Nov 2017 10:54:47 -0500
> 
> 
> > Can you tell more about this "scrambling"?  I don't understand what
> > that means.  An example where that happens would also be useful.
> 
> Original xml of INTRO TABLE BODY
> rendered as TABLE BODY INTRO
> because the table renderer bashed point.
>
> P.S. The xml files at https://github.com/KhronosGroup/OpenGL-Refpages/tree/master/es2.0
> often come out scrambled, it seems to depend on some race condition
> so any given file gets rendered correctly maybe 75% of the time.
> 
> P.P.S. I've seen no scrambled help docs since I applied this patch
> but can’t prove it really fixes every case, `save-window-excursion’
> is a can of worms as the doc string warns.  *Sigh* but at least my
> WebGL / OpenGL ES 2.0 context sensitive `describe-function’
> is unblocked.  AFAIK nobody else has done this but I’m quite
> out of touch, is there electric help for this annoying GPU API
> with parameter highlighting in the echo line and all?

Thanks, but could you possibly show a full recipe, starting from
"emacs -Q", assuming the above-mentioned files were downloaded?  I'd
like to be able to reproduce this on my machine and look into this.

And please keep the bug address on the CC list, so that this
discussion gets recorded by the issue tracker.

bug#29348: 26.0.50; shr.el heisenbug

[Devon Sean McCullough]

Downgrade from heisenbug to reproducible:
$ cd /tmp
$ cat > glFlush.xml <<\.
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "xhtml1-transitional.dtd">

<!-- saved from url=(0013)about:internet -->
<?xml-stylesheet type="text/xsl" href="mathml.xsl"?><html xmlns="http://www.w3.org/1999/xhtml" xmlns:pref="http://www.w3.org/2002/Math/preference" pref:renderer="mathplayer-dl"><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /><title>glFlush</title><meta name="generator" content="DocBook XSL Stylesheets V1.78.1" /></head><body><div class="refentry"><a id="glFlush"></a><div class="titlepage"></div><div class="refnamediv"><h2>Name</h2><p>glFlush — force execution of GL commands in finite time</p></div><div class="refsynopsisdiv"><h2>C Specification</h2><div class="funcsynopsis"><table border="0" class="funcprototype-table" summary="Function synopsis" style="cellspacing: 0; cellpadding: 0;"><tr><td><code class="funcdef">void <strong class="fsfunc">glFlush</strong>(</code></td><td> <var class="pdparam">void</var><code>)</code>;</td></tr></table><div class="funcprototype-spacer"> </div></div></div><div class="refsect1"><a id="description"></a><h2>Description</h2><p>
            Different GL implementations buffer commands in several different locations,
            including network buffers and the graphics accelerator itself.
            <code class="function">glFlush</code> empties all of these buffers,
            causing all issued commands to be executed as quickly as
            they are accepted by the actual rendering engine.
            Though this execution may not be completed in any particular
            time period,
            it does complete in finite time.
        </p><p>
            Because any GL program might be executed over a network,
            or on an accelerator that buffers commands,
            all programs should call <code class="function">glFlush</code> whenever they count on having
            all of their previously issued commands completed.
            For example,
            call <code class="function">glFlush</code> before waiting for user input that depends on
            the generated image.
        </p></div><div class="refsect1"><a id="notes"></a><h2>Notes</h2><p>
            <code class="function">glFlush</code> can return at any time.
            It does not wait until the execution of all previously
            issued GL commands is complete.
        </p></div><div class="refsect1"><a id="seealso"></a><h2>See Also</h2><p>
            <a class="citerefentry" href="glFinish.xml"><span class="citerefentry"><span class="refentrytitle">glFinish</span></span></a>
        </p></div><div class="refsect1"><a id="copyright"></a><h2>Copyright</h2><p>
            Copyright <span class="trademark"></span>© 1991-2006
            Silicon Graphics, Inc. This document is licensed under the SGI
            Free Software B License. For details, see
            <a class="ulink" href="http://oss.sgi.com/projects/FreeB/" target="_top">http://oss.sgi.com/projects/FreeB/</a>.
        </p></div></div></body></html>
.
$ Open -n -a Emacs --args -Q --chdir /tmp --eval '(let ((f "glFlush.xml")) (find-file f) (bury-buffer) (display-buffer f) (with-current-buffer f (shr-render-region (point-min) (point-max)) (not-modified)))'

bug#29348: 26.0.50; shr.el heisenbug

[Eli Zaretskii]

> From: Devon Sean McCullough <Emacs-Hacker2017@jovi.net>
> Date: Sun, 19 Nov 2017 23:18:02 -0500
> Cc: 29348@debbugs.gnu.org
> 
> Downgrade from heisenbug to reproducible:

Thanks, this reproducible recipe helped a lot.

Your original patch didn't fix the problem for me, but the one below
does.  Please see if it fixes your problems.

diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index 4ba452e..ab7240c 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -2289,8 +2289,10 @@ shr-render-td
 				  (<= (car (cdr attr)) width))
 			 (setq result (cdr attr)))))))
 	       result))
-	(let ((result (shr-render-td-1 dom width fill)))
+	(let* ((pt (point))
+               (result (shr-render-td-1 dom width fill)))
 	  (dom-set-attribute dom cache result)
+          (goto-char pt)
 	  result))))
 
 (defun shr-render-td-1 (dom width fill)

bug#29348: 26.0.50; shr.el heisenbug

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Your original patch didn't fix the problem for me, but the one below
> does.  Please see if it fixes your problems.
>
> diff --git a/lisp/net/shr.el b/lisp/net/shr.el
> index 4ba452e..ab7240c 100644
> --- a/lisp/net/shr.el
> +++ b/lisp/net/shr.el
> @@ -2289,8 +2289,10 @@ shr-render-td
>  				  (<= (car (cdr attr)) width))
>  			 (setq result (cdr attr)))))))
>  	       result))
> -	(let ((result (shr-render-td-1 dom width fill)))
> +	(let* ((pt (point))
> +               (result (shr-render-td-1 dom width fill)))
>  	  (dom-set-attribute dom cache result)
> +          (goto-char pt)
>  	  result))))

We didn't hear back in 19 weeks, so I'm assuming the patch worked, and
am closing this bug report.

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

bug#29348: 26.0.50; shr.el heisenbug

[Devon Sean McCullough]

cd /tmp
cURL -O http://jovi.net/shr.txz
tar -xp < shr.txz
cd shr
Open -n -a Emacs --args -Q --funcall toggle-debug-on-error --chdir /tmp/shr CNWebGL.js --load opengl-help.el --load shr-bug.el --funcall shr-trace

In the CNWebGL.js buffer, press the [f12] function key to view the next OpenGL call with documentation...
The first keypress or so should produce a good *Help* buffer with a proper title and name
but after three or four keypresses, mangled *Help* buffers will appear
with the C prototype at the top and the title at the bottom
because the table renderer bashed point in *Help*.

Example with annotated trace:

\f CNWebGL.js
	gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT);

\f *Help*
 void glClear(  GLbitfield mask);  
 
Parameters
... here follows the rest of the doc ... then finally the misplaced first few lines:
glClear

Name

glClear — clear buffers to preset values

C Specification

\f *trace-output*
======================================================================
1 -> (shr-render-region 1 6499) 20171119-164938.962420 •*Help*…❮6499❯⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…

| 2 -> (shr-pixel-column) 20171119-164938.963020 *Help*⊣❮1❯⊢• *temp*⊣-⊂2⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 -> (current-window-configuration) 20171119-164938.963084 *Help*⊣❮1❯⊢• *temp*⊣-⊂2⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 <- current-window-configuration: #<window-configuration> 20171119-164938.963137 *Help*⊣❮1❯⊢• *temp*⊣-⊂2⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 -> (set-window-configuration #<window-configuration>) 20171119-164938.963212 *Help*⊣❮1❯⊢⧉• *temp*⊣-❮2❯⊢
;;; moot
| | 3 <- set-window-configuration: t 20171119-164938.963316 *Help*⊣❮1❯⊢• *temp*⊣-⊂2⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| 2 <- shr-pixel-column: 7 20171119-164938.963362 *Help*⊣❮1❯⊢• *temp*⊣-⊂2⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…

| 2 -> (shr-pixel-column) 20171119-164938.963420 *Help*⊣❮1❯⊢• *temp*⊣* ⊂3⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 -> (current-window-configuration) 20171119-164938.963468 *Help*⊣❮1❯⊢• *temp*⊣* ⊂3⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 <- current-window-configuration: #<window-configuration> 20171119-164938.963513 *Help*⊣❮1❯⊢• *temp*⊣* ⊂3⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 -> (set-window-configuration #<window-configuration>) 20171119-164938.963570 *Help*⊣❮1❯⊢⧉• *temp*⊣* ❮3❯⊢
;;; moot
| | 3 <- set-window-configuration: t 20171119-164938.963660 *Help*⊣❮1❯⊢• *temp*⊣* ⊂3⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| 2 <- shr-pixel-column: 14 20171119-164938.963702 *Help*⊣❮1❯⊢• *temp*⊣* ⊂3⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…

| 2 -> (shr-render-td-1 (td nil (code ((class . "funcdef")) "void " (strong ((class . "fsfunc")) "glClear") "(")) 325 nil) 20171119-164938.963916 •*Help*…❮75❯⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 -> (current-window-configuration) 20171119-164938.964003 *Help*…❮75❯⊢• *temp*⊣void glClear(⊂14⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 <- current-window-configuration: #<window-configuration> 20171119-164938.964067 *Help*…❮75❯⊢• *temp*⊣void glClear(⊂14⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 -> (shr-pixel-buffer-width) 20171119-164938.964130 *Help*…❮75❯⊢⧉• *temp*⊣void glClear(❮14❯⊢
| | 3 <- shr-pixel-buffer-width: 71 20171119-164938.964196 *Help*…❮75❯⊢⧉• *temp*⊣void glClear(❮14❯⊢
| | 3 -> (set-window-configuration #<window-configuration>) 20171119-164938.964238 *Help*…❮75❯⊢⧉• *temp*⊣void glClear(❮14❯⊢

;;; ***BUG***
;;; Before
;;;	*Help* is visible in the other window
;;;	 *temp* is the current buffer in the selected window, presumably undisplayed but hypothetically "visible"
;;;	CNWebGL.js is neither current nor visible in any window
;;; *** `set-window-configuration' clobbers *Help* point from 75 at EoB to 1 at BoB ***
;;; After
;;;	*Help* is visible in the other window
;;;	 *temp* is the current buffer not visible in any window
;;;	CNWebGL.js is visible in the selected window
;;; ***BUG***

| | 3 <- set-window-configuration: t 20171119-164938.964342 *Help*⊣❮1❯glClear…• *temp*⊣void glClear(⊂14⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| 2 <- shr-render-td-1: (71 71 1 (#("void glClear(" 0 1 (face variable-pitch shr-indentation 0) 1 5 (face variable-pitch) 5 12 (face (variable-pitch bold)) 12 13 (face variable-pitch))) 1 nil nil) 20171119-164938.964426 •*Help*⊣❮1❯glClear…⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…

| 2 -> (shr-render-td-1 (td nil "GLbitfield " (var ((class . "pdparam")) "mask") (code nil ")") ";") 325 nil) 20171119-164938.964499 •*Help*⊣❮1❯glClear…⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 -> (current-window-configuration) 20171119-164938.964583 *Help*⊣❮1❯glClear…• *temp*…tfield mask);⊂18⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 <- current-window-configuration: #<window-configuration> 20171119-164938.964688 *Help*⊣❮1❯glClear…• *temp*…tfield mask);⊂18⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| | 3 -> (shr-pixel-buffer-width) 20171119-164938.964759 *Help*⊣❮1❯glClear…⧉• *temp*…tfield mask);❮18❯⊢
| | 3 <- shr-pixel-buffer-width: 91 20171119-164938.964837 *Help*⊣❮1❯glClear…⧉• *temp*…tfield mask);❮18❯⊢
| | 3 -> (set-window-configuration #<window-configuration>) 20171119-164938.964888 *Help*⊣❮1❯glClear…⧉• *temp*…tfield mask);❮18❯⊢
;;; equivocal
| | 3 <- set-window-configuration: t 20171119-164938.964996 *Help*⊣❮1❯glClear…• *temp*…tfield mask);⊂18⊃⊢⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…
| 2 <- shr-render-td-1: (91 91 1 (#("GLbitfield mask);" 0 1 (face variable-pitch shr-indentation 0) 1 11 (face variable-pitch) 11 15 (face variable-pitch) 15 16 (face variable-pitch) 16 17 (face variable-pitch))) 1 nil nil) 20171119-164938.965075 •*Help*⊣❮1❯glClear…⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…

1 <- shr-render-region: nil 20171119-164938.968888 •*Help*…❮1669❯glClear…⧉CNWebGL.js…	gl.❮4220❯clear(gl.COLO…