* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions @ 2015-10-22 4:04 Michael Welsh Duggan 2015-10-22 14:40 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: Michael Welsh Duggan @ 2015-10-22 4:04 UTC (permalink / raw) To: 21730 I don't have a recipe for this. Every now again I get the following in my *Messages* buffer: redisplay--pre-redisplay-functions: (args-out-of-range 0) I turned on debug-on-error, and got the following backtrace: Debugger entered--Lisp error: (args-out-of-range 0) get-char-property(0 cursor-sensor-functions) cursor-sensor--detect(#<window 224 on *Group*>) run-hook-with-args(cursor-sensor--detect #<window 224 on *Group*>) redisplay--pre-redisplay-functions(t) apply(redisplay--pre-redisplay-functions t) #[128 "\300\301\"\210\300\302\"\207" [apply redisplay--pre-redisplay-functions ignore nil] 4 nil nil](t) redisplay_internal\ \(C\ function\)() This happens infrequently, about a couple of times a day. I cannot make this happen on demand, but I can add debugging code if led in the correct direction. In GNU Emacs 25.0.50.1 (x86_64-pc-linux-gnu, X toolkit) of 2015-10-09 Repository revision: f7acc9ce65bc6c1080aa6c85618cf854ed0047ee Windowing system distributor 'The X.Org Foundation', version 11.0.11702000 System Description: Debian GNU/Linux unstable (sid) Configured using: 'configure --without-toolkit-scroll-bars --with-x-toolkit=lucid 'CFLAGS=-Og -ggdb3' --with-wide-int --with-gameuser=:staff' Configured features: XPM JPEG TIFF GIF PNG RSVG SOUND GPM DBUS GCONF GSETTINGS NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB LUCID X11 Important settings: value of $LANG: en_US.UTF-8 locale-coding-system: utf-8-unix Major mode: Dired by date Minor modes in effect: display-time-mode: t magit-auto-revert-mode: t shell-dirtrack-mode: t diff-auto-refine-mode: t tooltip-mode: t global-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 auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t buffer-read-only: t line-number-mode: t Recent messages: Saving /home/md5i/.newsrc.eld... Saving file /home/md5i/.newsrc.eld... Wrote /home/md5i/.newsrc.eld Saving /home/md5i/.newsrc.eld...done Checking new news... Reading active file from ~/Mail via nnml...done nnimap read 0k from localhost Reading active file via nndraft...done Checking new news...done Scanning for dabbrevs...done Load-path shadows: /usr/share/emacs/site-lisp/rst hides /usr/local/share/emacs/25.0.50/lisp/textmodes/rst Features: (shadow emacsbug gnus-fun calc-alg calc-ext calc-menu calc calc-loaddefs calc-macs doc-view css-mode smie image-mode pcmpl-unix debug js nxml-uchnm rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid nxml-mode nxml-outln nxml-rap nxml-glyph conf-mode cus-edit cus-start cus-load tramp-cache rect python tramp-sh json dabbrev gud imenu man time-stamp whitespace tabify two-column iso-transl make-mode dired-aux vc-dispatcher vc-hg cc-mode cc-fonts cc-guess cc-menus cc-cmds misearch multi-isearch gnus-dup shr-color color url-util url-parse url-vars shr dom subr-x browse-url flow-fill jka-compr mm-archive qp sort gnus-cite mail-extr gnus-async gnus-bcklg gnus-ml disp-table gnus-topic cursor-sensor utf-7 nndraft nnmh epa-file network-stream nsm starttls nnml gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-cache gnus-demon nntp gnutls nnir gnus-sum gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source tls utf7 netrc nnoo parse-time gnus-spec gnus-int gnus-range gnus-win gnus gnus-ems nnheader flyspell ispell yaml-mode uptimes pp descr-text time sieve-manage rng-loc rng-uri rng-parse rng-match rng-dt rng-util rng-pttrn nxml-parse nxml-ns nxml-enc xmltok nxml-util sgml-mode psvn wid-edit cl magit-key-mode magit view tramp tramp-compat auth-source eieio byte-opt bytecomp byte-compile cl-extra cconv eieio-core cl-macs gv gnus-util time-date password-cache tramp-loaddefs trampver shell pcomplete advice grep compile epa derived epg epg-config diff-mode autorevert filenotify git-rebase-mode thingatpt git-commit-mode server log-edit easy-mmode pcvs-util add-log mailcap message sendmail format-spec rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode mail-prsvr mailabbrev mail-utils gmm-utils mailheader lua-mode edmacro rx generated generic-x ediff-merg ediff-wind ediff-diff ediff-mult ediff-help ediff-init ediff-util ediff debian-el debian-el-loaddefs dired-x easymenu dired cc-styles cc-align cc-engine cc-vars cc-defs ange-ftp comint ansi-color ring cl-seq kmacro cl-loaddefs pcase cl-lib mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core 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 charscript case-table epa-hook jka-cmpr-hook help simple abbrev 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 dbusbind inotify dynamic-setting system-font-setting font-render-setting x-toolkit x multi-tty make-network-process emacs) Memory information: ((conses 16 885412 104614) (symbols 48 49376 228) (miscs 40 1199 2821) (strings 32 113818 15421) (string-bytes 1 3593164) (vectors 16 52064) (vector-slots 8 1440209 35827) (floats 8 761 638) (intervals 56 24560 199) (buffers 976 98) (heap 1024 103928 10949)) -- Michael Welsh Duggan (md5i@md5i.com) ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2015-10-22 4:04 bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions Michael Welsh Duggan @ 2015-10-22 14:40 ` Eli Zaretskii 2015-10-27 4:19 ` Michael Welsh Duggan 0 siblings, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2015-10-22 14:40 UTC (permalink / raw) To: Michael Welsh Duggan; +Cc: 21730 > From: Michael Welsh Duggan <mwd@md5i.com> > Date: Thu, 22 Oct 2015 00:04:06 -0400 > > I don't have a recipe for this. Every now again I get the following in > my *Messages* buffer: > > redisplay--pre-redisplay-functions: (args-out-of-range 0) > > I turned on debug-on-error, and got the following backtrace: > > Debugger entered--Lisp error: (args-out-of-range 0) > get-char-property(0 cursor-sensor-functions) > cursor-sensor--detect(#<window 224 on *Group*>) > run-hook-with-args(cursor-sensor--detect #<window 224 on *Group*>) > redisplay--pre-redisplay-functions(t) > apply(redisplay--pre-redisplay-functions t) > #[128 "\300\301\"\210\300\302\"\207" [apply > redisplay--pre-redisplay-functions ignore nil] 4 nil nil](t) > redisplay_internal\ \(C\ function\)() > > This happens infrequently, about a couple of times a day. I cannot make > this happen on demand, but I can add debugging code if led in the > correct direction. Thanks for reporting this. I'll try to help, although Stefan might have better ideas. From the backtrace it is obvious that the immediate reason for the problem is the call to 'get-char-property' with an invalid 1st argument of zero (it's a buffer position, so should be at least 1). Looking at 'cursor-sensor--detect', I see this: (defun cursor-sensor--detect (window) (unless cursor-sensor-inhibit (let* ((point (window-point window)) ;; It's often desirable to make the cursor-sensor-functions property ;; non-sticky on both ends, but that means get-pos-property might ;; never see it. (new (or (get-char-property point 'cursor-sensor-functions) (unless (bobp) (get-char-property (1- point) 'cursor-sensor-functions)))) So my guess is that the value of 'point' (the variable, not the function) is 1, i.e. beginning of buffer, and subtracting 1 from it produces that zero. Then the second call to 'get-char-property' barfs as expected. The code tries to protect itself against such a calamity by calling 'bobp', but that function looks at the buffer's value of point, which might be different from what 'window-point' returns. So my advice would be to replace the call to 'bobp' with an explicit test of the value of the local variable 'point', and see if that fixes the problem. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2015-10-22 14:40 ` Eli Zaretskii @ 2015-10-27 4:19 ` Michael Welsh Duggan 2015-10-31 13:07 ` Michael Welsh Duggan 0 siblings, 1 reply; 16+ messages in thread From: Michael Welsh Duggan @ 2015-10-27 4:19 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Michael Welsh Duggan, 21730 Eli Zaretskii <eliz@gnu.org> writes: >> From: Michael Welsh Duggan <mwd@md5i.com> >> Date: Thu, 22 Oct 2015 00:04:06 -0400 >> >> I don't have a recipe for this. Every now again I get the following in >> my *Messages* buffer: >> >> redisplay--pre-redisplay-functions: (args-out-of-range 0) >> >> I turned on debug-on-error, and got the following backtrace: >> >> Debugger entered--Lisp error: (args-out-of-range 0) >> get-char-property(0 cursor-sensor-functions) >> cursor-sensor--detect(#<window 224 on *Group*>) >> run-hook-with-args(cursor-sensor--detect #<window 224 on *Group*>) >> redisplay--pre-redisplay-functions(t) >> apply(redisplay--pre-redisplay-functions t) >> #[128 "\300\301\"\210\300\302\"\207" [apply >> redisplay--pre-redisplay-functions ignore nil] 4 nil nil](t) >> redisplay_internal\ \(C\ function\)() >> >> This happens infrequently, about a couple of times a day. I cannot make >> this happen on demand, but I can add debugging code if led in the >> correct direction. > > Thanks for reporting this. I'll try to help, although Stefan might > have better ideas. > > From the backtrace it is obvious that the immediate reason for the > problem is the call to 'get-char-property' with an invalid 1st argument > of zero (it's a buffer position, so should be at least 1). > > Looking at 'cursor-sensor--detect', I see this: > > (defun cursor-sensor--detect (window) > (unless cursor-sensor-inhibit > (let* ((point (window-point window)) > ;; It's often desirable to make the cursor-sensor-functions property > ;; non-sticky on both ends, but that means get-pos-property might > ;; never see it. > (new (or (get-char-property point 'cursor-sensor-functions) > (unless (bobp) > (get-char-property (1- point) 'cursor-sensor-functions)))) > > So my guess is that the value of 'point' (the variable, not the > function) is 1, i.e. beginning of buffer, and subtracting 1 from it > produces that zero. Then the second call to 'get-char-property' barfs > as expected. > > The code tries to protect itself against such a calamity by calling > 'bobp', but that function looks at the buffer's value of point, which > might be different from what 'window-point' returns. > > So my advice would be to replace the call to 'bobp' with an explicit > test of the value of the local variable 'point', and see if that fixes > the problem. I've changed the (bobp) to (= point 1). I'll run it for a couple of weeks and report back. -- Michael Welsh Duggan (md5i@md5i.com) ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2015-10-27 4:19 ` Michael Welsh Duggan @ 2015-10-31 13:07 ` Michael Welsh Duggan 2015-10-31 13:32 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: Michael Welsh Duggan @ 2015-10-31 13:07 UTC (permalink / raw) To: Eli Zaretskii, 21730 Michael Welsh Duggan <mwd@md5i.com> writes: > Eli Zaretskii <eliz@gnu.org> writes: > >> From the backtrace it is obvious that the immediate reason for the >> problem is the call to 'get-char-property' with an invalid 1st argument >> of zero (it's a buffer position, so should be at least 1). >> >> Looking at 'cursor-sensor--detect', I see this: >> >> (defun cursor-sensor--detect (window) >> (unless cursor-sensor-inhibit >> (let* ((point (window-point window)) >> ;; It's often desirable to make the cursor-sensor-functions property >> ;; non-sticky on both ends, but that means get-pos-property might >> ;; never see it. >> (new (or (get-char-property point 'cursor-sensor-functions) >> (unless (bobp) >> (get-char-property (1- point) 'cursor-sensor-functions)))) >> >> So my guess is that the value of 'point' (the variable, not the >> function) is 1, i.e. beginning of buffer, and subtracting 1 from it >> produces that zero. Then the second call to 'get-char-property' barfs >> as expected. >> >> The code tries to protect itself against such a calamity by calling >> 'bobp', but that function looks at the buffer's value of point, which >> might be different from what 'window-point' returns. >> >> So my advice would be to replace the call to 'bobp' with an explicit >> test of the value of the local variable 'point', and see if that fixes >> the problem. > > I've changed the (bobp) to (= point 1). I'll run it for a couple of > weeks and report back. This change seems to have done the trick. I have not encountered the error in several days. -- Michael Welsh Duggan (md5i@md5i.com) ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2015-10-31 13:07 ` Michael Welsh Duggan @ 2015-10-31 13:32 ` Eli Zaretskii 2016-09-14 17:01 ` Philipp Stephani [not found] ` <CAArVCkTA5_hxvxszdYX1QWSoG382zg+mW=4U3uhiXmTBcPCSgw@mail.gmail.com> 0 siblings, 2 replies; 16+ messages in thread From: Eli Zaretskii @ 2015-10-31 13:32 UTC (permalink / raw) To: Michael Welsh Duggan; +Cc: 21730-done > From: Michael Welsh Duggan <mwd@md5i.com> > Date: Sat, 31 Oct 2015 09:07:05 -0400 > > > I've changed the (bobp) to (= point 1). I'll run it for a couple of > > weeks and report back. > > This change seems to have done the trick. I have not encountered the > error in several days. Thanks for testing. I pushed the change, and I'm arking this bug as done. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2015-10-31 13:32 ` Eli Zaretskii @ 2016-09-14 17:01 ` Philipp Stephani [not found] ` <CAArVCkTA5_hxvxszdYX1QWSoG382zg+mW=4U3uhiXmTBcPCSgw@mail.gmail.com> 1 sibling, 0 replies; 16+ messages in thread From: Philipp Stephani @ 2016-09-14 17:01 UTC (permalink / raw) To: Eli Zaretskii, 21730, mwd; +Cc: 21730-done [-- Attachment #1: Type: text/plain, Size: 2146 bytes --] Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 31. Okt. 2015 um 14:34 Uhr: > > From: Michael Welsh Duggan <mwd@md5i.com> > > Date: Sat, 31 Oct 2015 09:07:05 -0400 > > > > > I've changed the (bobp) to (= point 1). I'll run it for a couple of > > > weeks and report back. > > > > This change seems to have done the trick. I have not encountered the > > error in several days. > > Thanks for testing. I pushed the change, and I'm arking this bug as > done. > > > This change was reverted in 76ef52267cf887e3e1aa6d25b3b16dd0601dd459. It also doesn't seem correct. cursor-sensor--detect is only used in pre-redisplay-functions, and the documentation of that variable says: "Hook run just before redisplay. It is called in each window that is to be redisplayed. It takes one argument, which is the window that will be redisplayed. When run, the ‘current-buffer’ is set to the buffer displayed in that window." That means that (bobp) is correct and (= point 1) cannot give a different result, unless narrowing is in effect (then only bobp is correct). Given that replacing (bobp) with (= point 1) does solve this bug, the documentation of pre-redisplay-functions must be incorrect, i.e. the current buffer is not the buffer of the window passed as argument. I think the only way how this can happen is that a previous entry in pre-redisplay-functions has changed the current buffer. Probably the implementation of redisplay--pre-redisplay-functions should be changed from (with-current-buffer (window-buffer win) (run-hook-with-args 'pre-redisplay-functions win)) to (run-hook-wrapped 'pre-redisplay-functions (lambda (func) (with-current-buffer (window-buffer win) (funcall func win) nil)) or so. So we might try the following: Replace redisplay--pre-redisplay-functions (and indeed, all hooks that document anything about the current buffer) as above. Then add (cl-assert (eq (current-buffer) (window-buffer window)) to cursor-sensor--detect and remove code such as (window-point window), because that must be equal to (point). WDYT? Or am I misunderstanding something? [-- Attachment #2: Type: text/html, Size: 2969 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <CAArVCkTA5_hxvxszdYX1QWSoG382zg+mW=4U3uhiXmTBcPCSgw@mail.gmail.com>]
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions [not found] ` <CAArVCkTA5_hxvxszdYX1QWSoG382zg+mW=4U3uhiXmTBcPCSgw@mail.gmail.com> @ 2016-09-14 17:09 ` Eli Zaretskii 2016-09-14 18:43 ` Philipp Stephani 2016-09-14 19:11 ` Philipp Stephani 1 sibling, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2016-09-14 17:09 UTC (permalink / raw) To: Philipp Stephani; +Cc: 21730, mwd > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Wed, 14 Sep 2016 16:54:23 +0000 > > Thanks for testing. I pushed the change, and I'm arking this bug as > done. > > This change was reverted in 76ef52267cf887e3e1aa6d25b3b16dd0601dd459. Too bad people revert changes that fixed a bug without making sure they don't reintroduce the same bug again. Now Emacs 25.1 will be released with this bug. > Given that replacing (bobp) with (= point 1) does solve this bug, the documentation of pre-redisplay-functions > must be incorrect, i.e. the current buffer is not the buffer of the window passed as argument. I think the only > way how this can happen is that a previous entry in pre-redisplay-functions has changed the current buffer. > Probably the implementation of redisplay--pre-redisplay-functions should be changed from > (with-current-buffer (window-buffer win) > (run-hook-with-args 'pre-redisplay-functions win)) > to > (run-hook-wrapped 'pre-redisplay-functions > (lambda (func) (with-current-buffer (window-buffer win) > (funcall func win) > nil)) > or so. > > So we might try the following: Replace redisplay--pre-redisplay-functions (and indeed, all hooks that > document anything about the current buffer) as above. Then add (cl-assert (eq (current-buffer) > (window-buffer window)) to cursor-sensor--detect and remove code such as (window-point window), because > that must be equal to (point). WDYT? Or am I misunderstanding something? Since the problem that caused the patch to be reverted was with narrowing, why not simply use (= point (with-current-buffer (window-buffer window) (point-min))) instead of (= point 1) ? Anyway, my role in this bug was just to facilitate the debugging and push a commit, I didn't really look at the issue deeper than what you see in the discussion. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2016-09-14 17:09 ` Eli Zaretskii @ 2016-09-14 18:43 ` Philipp Stephani 2016-09-14 19:23 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: Philipp Stephani @ 2016-09-14 18:43 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21730, mwd [-- Attachment #1: Type: text/plain, Size: 1947 bytes --] Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 14. Sep. 2016 um 19:11 Uhr: > > > Given that replacing (bobp) with (= point 1) does solve this bug, the > documentation of pre-redisplay-functions > > must be incorrect, i.e. the current buffer is not the buffer of the > window passed as argument. I think the only > > way how this can happen is that a previous entry in > pre-redisplay-functions has changed the current buffer. > > Probably the implementation of redisplay--pre-redisplay-functions should > be changed from > > (with-current-buffer (window-buffer win) > > (run-hook-with-args 'pre-redisplay-functions win)) > > to > > (run-hook-wrapped 'pre-redisplay-functions > > (lambda (func) (with-current-buffer (window-buffer win) > > (funcall func win) > > nil)) > > or so. > > > > So we might try the following: Replace > redisplay--pre-redisplay-functions (and indeed, all hooks that > > document anything about the current buffer) as above. Then add > (cl-assert (eq (current-buffer) > > (window-buffer window)) to cursor-sensor--detect and remove code such as > (window-point window), because > > that must be equal to (point). WDYT? Or am I misunderstanding something? > > Since the problem that caused the patch to be reverted was with > narrowing, why not simply use > > (= point (with-current-buffer (window-buffer window) (point-min))) > > instead of > > (= point 1) > > ? > > This has the same issues as described above. There's more code in cursor-sensor--detect that assumes (eq (current-buffer) (window-buffer window)), e.g. the form (get-char-property point 'cursor-sensor-functions). Therefore (= point (with-current-buffer (window-buffer window) (point-min))) must be the same as (bobp). Another mystery is the stack trace entry "run-hook-with-args(cursor-sensor--detect #<window 224 on *Group*>)". How can cursor-sensor--detect be the first argument to run-hook-with-args? Shouldn't it always be pre-redisplay-functions? [-- Attachment #2: Type: text/html, Size: 2934 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2016-09-14 18:43 ` Philipp Stephani @ 2016-09-14 19:23 ` Eli Zaretskii 2016-09-14 19:48 ` Philipp Stephani 0 siblings, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2016-09-14 19:23 UTC (permalink / raw) To: Philipp Stephani; +Cc: 21730, mwd > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Wed, 14 Sep 2016 18:43:13 +0000 > Cc: 21730@debbugs.gnu.org, mwd@md5i.com > > Since the problem that caused the patch to be reverted was with > narrowing, why not simply use > > (= point (with-current-buffer (window-buffer window) (point-min))) > > instead of > > (= point 1) > > ? > > This has the same issues as described above. There's more code in cursor-sensor--detect that assumes (eq > (current-buffer) (window-buffer window)), e.g. the form (get-char-property point 'cursor-sensor-functions). > Therefore (= point (with-current-buffer (window-buffer window) (point-min))) must be the same as (bobp). > Another mystery is the stack trace entry "run-hook-with-args(cursor-sensor--detect #<window 224 on > *Group*>)". How can cursor-sensor--detect be the first argument to run-hook-with-args? Shouldn't it always > be pre-redisplay-functions? If we want to dig deeper, I think we should first understand why bop doesn't work. Is it indeed because the window's buffer is not the current buffer when the hook is called? ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2016-09-14 19:23 ` Eli Zaretskii @ 2016-09-14 19:48 ` Philipp Stephani 2016-09-15 14:22 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: Philipp Stephani @ 2016-09-14 19:48 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21730, mwd [-- Attachment #1: Type: text/plain, Size: 1497 bytes --] Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 14. Sep. 2016 um 21:24 Uhr: > > From: Philipp Stephani <p.stephani2@gmail.com> > > Date: Wed, 14 Sep 2016 18:43:13 +0000 > > Cc: 21730@debbugs.gnu.org, mwd@md5i.com > > > > Since the problem that caused the patch to be reverted was with > > narrowing, why not simply use > > > > (= point (with-current-buffer (window-buffer window) (point-min))) > > > > instead of > > > > (= point 1) > > > > ? > > > > This has the same issues as described above. There's more code in > cursor-sensor--detect that assumes (eq > > (current-buffer) (window-buffer window)), e.g. the form > (get-char-property point 'cursor-sensor-functions). > > Therefore (= point (with-current-buffer (window-buffer window) > (point-min))) must be the same as (bobp). > > Another mystery is the stack trace entry > "run-hook-with-args(cursor-sensor--detect #<window 224 on > > *Group*>)". How can cursor-sensor--detect be the first argument to > run-hook-with-args? Shouldn't it always > > be pre-redisplay-functions? > > If we want to dig deeper, I think we should first understand why bop > doesn't work. Is it indeed because the window's buffer is not the > current buffer when the hook is called? > Unfortunately the error doesn't trigger consistently. Maybe we are lucky and somebody manages to detect such an issue. I'd also suggest to add (cl-assert (eq (current-buffer) (window-buffer window))) to cursor-sensor--detect, because the function relies on this equality. [-- Attachment #2: Type: text/html, Size: 2664 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2016-09-14 19:48 ` Philipp Stephani @ 2016-09-15 14:22 ` Eli Zaretskii 0 siblings, 0 replies; 16+ messages in thread From: Eli Zaretskii @ 2016-09-15 14:22 UTC (permalink / raw) To: Philipp Stephani; +Cc: 21730, mwd > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Wed, 14 Sep 2016 19:48:23 +0000 > Cc: 21730@debbugs.gnu.org, mwd@md5i.com > > If we want to dig deeper, I think we should first understand why bop > doesn't work. Is it indeed because the window's buffer is not the > current buffer when the hook is called? > > Unfortunately the error doesn't trigger consistently. Maybe we are lucky and somebody manages to detect > such an issue. > > I'd also suggest to add > (cl-assert (eq (current-buffer) (window-buffer window))) > to cursor-sensor--detect, because the function relies on this equality. Michael, could you please look some more into this? I think we need to understand the reason why bobp doesn't work, although the hook was supposed to be called with the window's buffer the current buffer. TIA ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions [not found] ` <CAArVCkTA5_hxvxszdYX1QWSoG382zg+mW=4U3uhiXmTBcPCSgw@mail.gmail.com> 2016-09-14 17:09 ` Eli Zaretskii @ 2016-09-14 19:11 ` Philipp Stephani 2016-09-14 19:25 ` Eli Zaretskii 1 sibling, 1 reply; 16+ messages in thread From: Philipp Stephani @ 2016-09-14 19:11 UTC (permalink / raw) To: Eli Zaretskii, 21730, mwd [-- Attachment #1.1: Type: text/plain, Size: 1943 bytes --] Philipp Stephani <p.stephani2@gmail.com> schrieb am Mi., 14. Sep. 2016 um 18:54 Uhr: > Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 31. Okt. 2015 um 14:34 Uhr: > > > From: Michael Welsh Duggan <mwd@md5i.com> > > Date: Sat, 31 Oct 2015 09:07:05 -0400 > > > > > I've changed the (bobp) to (= point 1). I'll run it for a couple of > > > weeks and report back. > > > > This change seems to have done the trick. I have not encountered the > > error in several days. > > Thanks for testing. I pushed the change, and I'm arking this bug as > done. > > > > This change was reverted in 76ef52267cf887e3e1aa6d25b3b16dd0601dd459. > It also doesn't seem correct. cursor-sensor--detect is only used in > pre-redisplay-functions, and the documentation of that variable says: > "Hook run just before redisplay. > It is called in each window that is to be redisplayed. It takes one > argument, > which is the window that will be redisplayed. When run, the > ‘current-buffer’ > is set to the buffer displayed in that window." > That means that (bobp) is correct and (= point 1) cannot give a different > result, unless narrowing is in effect (then only bobp is correct). > Given that replacing (bobp) with (= point 1) does solve this bug, the > documentation of pre-redisplay-functions must be incorrect, i.e. the > current buffer is not the buffer of the window passed as argument. I think > the only way how this can happen is that a previous entry in > pre-redisplay-functions has changed the current buffer. Probably the > implementation of redisplay--pre-redisplay-functions should be changed from > (with-current-buffer (window-buffer win) > (run-hook-with-args 'pre-redisplay-functions win)) > to > (run-hook-wrapped 'pre-redisplay-functions > (lambda (func) (with-current-buffer (window-buffer win) > (funcall func win) > nil)) > or so. > I've attached a patch for this. [-- Attachment #1.2: Type: text/html, Size: 3500 bytes --] [-- Attachment #2: 0001-Restore-buffer-in-pre-redisplay-functions.txt --] [-- Type: text/plain, Size: 3499 bytes --] From 361a0488a67ffc601e207626b25327bf4868d24a Mon Sep 17 00:00:00 2001 From: Philipp Stephani <phst@google.com> Date: Wed, 14 Sep 2016 21:08:07 +0200 Subject: [PATCH] Restore buffer in `pre-redisplay-functions' Previously, a function in `pre-redisplay-functions' could change the current buffer, breaking the contract of `pre-redisplay-functions' that the current buffer is always the buffer displayed in the argument window. * lisp/simple.el (redisplay--pre-redisplay-functions): Fix behavior when a function in `pre-redisplay-functions' changes the current buffer. * test/lisp/simple-tests.el (pre-redisplay-function--current-buffer): Test for fixed behavior. --- lisp/simple.el | 16 ++++++++++------ test/lisp/simple-tests.el | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/lisp/simple.el b/lisp/simple.el index 04a525c..9a4690e 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -5425,13 +5425,17 @@ pre-redisplay-functions is set to the buffer displayed in that window.") (defun redisplay--pre-redisplay-functions (windows) + (cond + ((null windows) (setq windows (list (selected-window)))) + ((nlistp windows) (setq windows (window-list-1 nil nil t)))) (with-demoted-errors "redisplay--pre-redisplay-functions: %S" - (if (null windows) - (with-current-buffer (window-buffer (selected-window)) - (run-hook-with-args 'pre-redisplay-functions (selected-window))) - (dolist (win (if (listp windows) windows (window-list-1 nil nil t))) - (with-current-buffer (window-buffer win) - (run-hook-with-args 'pre-redisplay-functions win)))))) + (dolist (win windows) + (run-hook-wrapped + 'pre-redisplay-functions + (lambda (func) + (with-current-buffer (window-buffer win) + (funcall func win)) + nil))))) (add-function :before pre-redisplay-function #'redisplay--pre-redisplay-functions) diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el index d022240..da7a03d 100644 --- a/test/lisp/simple-tests.el +++ b/test/lisp/simple-tests.el @@ -374,5 +374,33 @@ simple-test-undo-with-switched-buffer (undo) (point))))) +\f +;;; `pre-redisplay-function' +(ert-deftest pre-redisplay-function--current-buffer () + "Verify that the functions in `pre-redisplay-functions' are called with the correct `current-buffer'. +See Bug#21730." + (with-temp-buffer + (let ((buffer-1 (current-buffer))) + (with-temp-buffer + (let ((buffer-2 (current-buffer)) + (pre-redisplay-functions pre-redisplay-functions) + (hook-1-calls 0) + (hook-2-calls 0)) + (add-hook 'pre-redisplay-functions + (lambda (window) + (should (equal (current-buffer) (window-buffer window))) + (cl-incf hook-1-calls) + (set-buffer buffer-1)) + :append) + (add-hook 'pre-redisplay-functions + (lambda (window) + (should (equal (current-buffer) (window-buffer window))) + (cl-incf hook-2-calls)) + :append) + (funcall pre-redisplay-function nil) + (should (equal (current-buffer) buffer-2)) + (should (equal hook-1-calls 1)) + (should (equal hook-2-calls 1))))))) + (provide 'simple-test) ;;; simple-test.el ends here -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2016-09-14 19:11 ` Philipp Stephani @ 2016-09-14 19:25 ` Eli Zaretskii 2017-07-25 2:06 ` Sergio Durigan Junior 0 siblings, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2016-09-14 19:25 UTC (permalink / raw) To: Philipp Stephani; +Cc: 21730, mwd > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Wed, 14 Sep 2016 19:11:57 +0000 > > (with-current-buffer (window-buffer win) > (run-hook-with-args 'pre-redisplay-functions win)) > to > (run-hook-wrapped 'pre-redisplay-functions > (lambda (func) (with-current-buffer (window-buffer win) > (funcall func win) > nil)) > or so. > > I've attached a patch for this. Thanks. But we are going to entertain such changes, I think we really should be sure the issue with the current buffer is the culprit. See my other message a few moments ago. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2016-09-14 19:25 ` Eli Zaretskii @ 2017-07-25 2:06 ` Sergio Durigan Junior 2017-07-25 14:21 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: Sergio Durigan Junior @ 2017-07-25 2:06 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21730, mwd, Philipp Stephani [-- Attachment #1: Type: text/plain, Size: 2773 bytes --] On Wednesday, September 14 2016, Eli Zaretskii wrote: [ Resending because I have now really unarchived the bug. Sorry about the dup. ] >> From: Philipp Stephani <p.stephani2@gmail.com> >> Date: Wed, 14 Sep 2016 19:11:57 +0000 >> >> (with-current-buffer (window-buffer win) >> (run-hook-with-args 'pre-redisplay-functions win)) >> to >> (run-hook-wrapped 'pre-redisplay-functions >> (lambda (func) (with-current-buffer (window-buffer win) >> (funcall func win) >> nil)) >> or so. >> >> I've attached a patch for this. > > Thanks. But we are going to entertain such changes, I think we really > should be sure the issue with the current buffer is the culprit. See > my other message a few moments ago. It is pretty hard to diagnose and investigate this issue, but I think I've managed to progress a bit. The problem doesn't seem to be on bobp, and I happen to share Philipp's opinion that the real issue is the assumption that cursor-sensor--detect will be at the right buffer when it's not. Here, let me show you. On my machine, this error always happens when I'm running Gnus in one buffer (via emacsclient), and while I am visiting a file in another buffer, and in another window (i.e., another emacsclient). The backtrace I get is: Debugger entered--Lisp error: (args-out-of-range 0) get-char-property(0 cursor-sensor-functions) cursor-sensor--detect(#<window 5650 on *Group*>) ^^^^^^^^^^^^^^^^^^^^^^^^^ run-hook-with-args(cursor-sensor--detect #<window 5650 on *Group*>) redisplay--pre-redisplay-functions(t) apply(redisplay--pre-redisplay-functions t) [ lots of other functions not interesting for us ] I took the liberty to mark the interesting part above, which is the argument to cursor-sensor--detect. As you can notice, it's working on the *Group* window, from Gnus. Now, the interesting part that I noticed and was able to verify is that this only happens when (a) I am not working in that buffer, (b) the point is at (point-min) at that buffer, but (c) the point is not (point-min) at the buffer I'm currently in. In this case, Emacs does some wrong calculations with point and ends up with the value of 0, which makes sense if you consider that (window-point) of that window is 1, but (bobp) is not t (because, as I explained, I'm working in another buffer, and the value of point there is not 1). With all that said, I came to the conclusion that Phillip's rationale makes sense and therefore his patch seems to be the best shot we have at fixing this annoying bug. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2017-07-25 2:06 ` Sergio Durigan Junior @ 2017-07-25 14:21 ` Eli Zaretskii 2017-07-25 23:15 ` Sergio Durigan Junior 0 siblings, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2017-07-25 14:21 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: 21730, mwd, p.stephani2 > From: Sergio Durigan Junior <sergiodj@sergiodj.net> > Cc: Philipp Stephani <p.stephani2@gmail.com>, 21730@debbugs.gnu.org, mwd@md5i.com > Date: Mon, 24 Jul 2017 22:06:56 -0400 > > Debugger entered--Lisp error: (args-out-of-range 0) > get-char-property(0 cursor-sensor-functions) > cursor-sensor--detect(#<window 5650 on *Group*>) > ^^^^^^^^^^^^^^^^^^^^^^^^^ > run-hook-with-args(cursor-sensor--detect #<window 5650 on *Group*>) > redisplay--pre-redisplay-functions(t) > apply(redisplay--pre-redisplay-functions t) > [ lots of other functions not interesting for us ] > > I took the liberty to mark the interesting part above, which is the > argument to cursor-sensor--detect. As you can notice, it's working on > the *Group* window, from Gnus. Now, the interesting part that I noticed > and was able to verify is that this only happens when (a) I am not > working in that buffer, (b) the point is at (point-min) at that buffer, > but (c) the point is not (point-min) at the buffer I'm currently in. In > this case, Emacs does some wrong calculations with point and ends up > with the value of 0, which makes sense if you consider that > (window-point) of that window is 1, but (bobp) is not t (because, as I > explained, I'm working in another buffer, and the value of point there > is not 1). > > With all that said, I came to the conclusion that Phillip's rationale > makes sense and therefore his patch seems to be the best shot we have at > fixing this annoying bug. Thanks for following up. Phillip's hypothesis assumes that there's another function in pre-redisplay-function, which is called before cursor-sensor--detect and switches to another buffer, leaving us there. Do you indeed have other functions on that list? If so, what are they? I'm asking because I'd like to be sure we are on the right track with the root cause. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions 2017-07-25 14:21 ` Eli Zaretskii @ 2017-07-25 23:15 ` Sergio Durigan Junior 0 siblings, 0 replies; 16+ messages in thread From: Sergio Durigan Junior @ 2017-07-25 23:15 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21730, mwd, p.stephani2 On Tuesday, July 25 2017, Eli Zaretskii wrote: >> From: Sergio Durigan Junior <sergiodj@sergiodj.net> >> Cc: Philipp Stephani <p.stephani2@gmail.com>, 21730@debbugs.gnu.org, mwd@md5i.com >> Date: Mon, 24 Jul 2017 22:06:56 -0400 >> >> Debugger entered--Lisp error: (args-out-of-range 0) >> get-char-property(0 cursor-sensor-functions) >> cursor-sensor--detect(#<window 5650 on *Group*>) >> ^^^^^^^^^^^^^^^^^^^^^^^^^ >> run-hook-with-args(cursor-sensor--detect #<window 5650 on *Group*>) >> redisplay--pre-redisplay-functions(t) >> apply(redisplay--pre-redisplay-functions t) >> [ lots of other functions not interesting for us ] >> >> I took the liberty to mark the interesting part above, which is the >> argument to cursor-sensor--detect. As you can notice, it's working on >> the *Group* window, from Gnus. Now, the interesting part that I noticed >> and was able to verify is that this only happens when (a) I am not >> working in that buffer, (b) the point is at (point-min) at that buffer, >> but (c) the point is not (point-min) at the buffer I'm currently in. In >> this case, Emacs does some wrong calculations with point and ends up >> with the value of 0, which makes sense if you consider that >> (window-point) of that window is 1, but (bobp) is not t (because, as I >> explained, I'm working in another buffer, and the value of point there >> is not 1). >> >> With all that said, I came to the conclusion that Phillip's rationale >> makes sense and therefore his patch seems to be the best shot we have at >> fixing this annoying bug. > > Thanks for following up. > > Phillip's hypothesis assumes that there's another function in > pre-redisplay-function, which is called before cursor-sensor--detect > and switches to another buffer, leaving us there. Do you indeed have > other functions on that list? If so, what are they? You know, that's a good question. After looking into what I have here, I don't see any other function being called before pre-redisplay-functions. Everything seems fine and I see the call to with-current-buffer before calling run-hook-with-args, which means that, unless there is a bug somewhere, I don't see how the buffer could have changed. > I'm asking because I'd like to be sure we are on the right track with > the root cause. Sure thing. Well, if you ask me, I'm convinced that what I explained in my last message is what happens, but I still don't know *why* it happens. I'll see if I manage to investigate this a bit more. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-07-25 23:15 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-22 4:04 bug#21730: 25.0.50; Random errors in redisplay--pre-redisplay-functions Michael Welsh Duggan 2015-10-22 14:40 ` Eli Zaretskii 2015-10-27 4:19 ` Michael Welsh Duggan 2015-10-31 13:07 ` Michael Welsh Duggan 2015-10-31 13:32 ` Eli Zaretskii 2016-09-14 17:01 ` Philipp Stephani [not found] ` <CAArVCkTA5_hxvxszdYX1QWSoG382zg+mW=4U3uhiXmTBcPCSgw@mail.gmail.com> 2016-09-14 17:09 ` Eli Zaretskii 2016-09-14 18:43 ` Philipp Stephani 2016-09-14 19:23 ` Eli Zaretskii 2016-09-14 19:48 ` Philipp Stephani 2016-09-15 14:22 ` Eli Zaretskii 2016-09-14 19:11 ` Philipp Stephani 2016-09-14 19:25 ` Eli Zaretskii 2017-07-25 2:06 ` Sergio Durigan Junior 2017-07-25 14:21 ` Eli Zaretskii 2017-07-25 23:15 ` Sergio Durigan Junior
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).