bug#71774: 31.0.50; Hash table weakness broken?

bug#71774: 31.0.50; Hash table weakness broken?

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

With Emacs -Q evaluate the following forms in *scratch* step by step:


(defvar table (make-hash-table :weakness t))

(puthash t (split-window) table)

(delete-other-windows)

(garbage-collect)

(gethash t table)


The last form evaluates to #<window 7> here.  In all prior Emacs
versions I have it evaluates to nil.  IIUC this means that either hash
table weakness is broken or there is a bug in the collector.  To exclude
that the behavior is specific to the window handling code, note that the
recipe you can find here

https://nullprogram.com/blog/2012/12/17/

is now broken too.


In GNU Emacs 31.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version
  3.24.38, cairo version 1.16.0) of 2024-06-25 built on restno
Repository revision: b9da5ee06f2e4ae807336dd6a641ae797831d097
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
  'configure --with-gif=ifavailable --with-tiff=ifavailable
  --with-gnutls=no --without-pop --enable-gcc-warnings=warn-only
  --enable-checking=yes,glyphs,structs
  --enable-check-lisp-object-type=yes 'CFLAGS=-O0 -g3 -no-pie
  -Wno-missing-braces''

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GSETTINGS HARFBUZZ JPEG LIBSELINUX MODULES
NOTIFY INOTIFY PDUMPER PNG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LANG: de_AT.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   show-paren-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
   minibuffer-regexp-mode: t
   line-number-mode: t
   indent-tabs-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd touch-screen tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine 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 emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo gtk
x-toolkit xinput2 x multi-tty move-toolbar make-network-process emacs)

Memory information:
((conses 16 39683 9012) (symbols 48 5369 0) (strings 32 12780 1387)
  (string-bytes 1 312604) (vectors 16 9423)
  (vector-slots 8 113146 8563) (floats 8 25 29) (intervals 56 255 0)
  (buffers 984 11))

bug#71774: 31.0.50; Hash table weakness broken?

[Eli Zaretskii]

> Date: Tue, 25 Jun 2024 15:57:55 +0200
> From:  martin rudalics via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> With Emacs -Q evaluate the following forms in *scratch* step by step:
> 
> 
> (defvar table (make-hash-table :weakness t))
> 
> (puthash t (split-window) table)
> 
> (delete-other-windows)
> 
> (garbage-collect)
> 
> (gethash t table)
> 
> 
> The last form evaluates to #<window 7> here.  In all prior Emacs
> versions I have it evaluates to nil.  IIUC this means that either hash
> table weakness is broken or there is a bug in the collector.  To exclude
> that the behavior is specific to the window handling code, note that the
> recipe you can find here
> 
> https://nullprogram.com/blog/2012/12/17/
> 
> is now broken too.

Mattias, could you perhaps look into this?  I presume the same happens
on the emacs-30 branch, which makes the fix more important.

Thanks.

bug#71774: 31.0.50; Hash table weakness broken?

[Mattias Engdegård]

> The last form evaluates to #<window 7> here.  In all prior Emacs
> versions I have it evaluates to nil.  IIUC this means that either hash
> table weakness is broken or there is a bug in the collector.  To exclude
> that the behavior is specific to the window handling code, note that the
> recipe you can find here
> 
> https://nullprogram.com/blog/2012/12/17/
> 
> is now broken too.

Thanks for the report, but I wouldn't read too much into that. There could always be a stray reference to an object that is otherwise expected to be dead. Sometimes because of the conservative roots, sometimes from internal state we don't think about.

There is never a guarantee that any particular object is treated as dead by the GC. It's more of a general ambition.

As far as I can tell, it works as expected. Sometimes there is a stubborn value that just doesn't seem to want to be collected, but it's probably just a stray value on the stack, or a hidden reference that got stashed somewhere by the REPL.

bug#71774: 31.0.50; Hash table weakness broken?

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > Thanks for the report, but I wouldn't read too much into that. There
 > could always be a stray reference to an object that is otherwise
 > expected to be dead. Sometimes because of the conservative roots,
 > sometimes from internal state we don't think about.

I tried once more with some 10 runs of automatic garbage collection
triggered in between and the result is the same.  Neither ambiguous
roots nor internal state can explain that new behavior IMHO.

 > There is never a guarantee that any particular object is treated as
 > dead by the GC. It's more of a general ambition.

A dead object that survives several collection cycles qualifies as a
leak in my view.

 > As far as I can tell, it works as expected. Sometimes there is a
 > stubborn value that just doesn't seem to want to be collected, but
 > it's probably just a stray value on the stack, or a hidden reference
 > that got stashed somewhere by the REPL.

This does not explain the change in behavior from all versions from
Emacs 24 up to and including Emacs 29 here to the current behavior.  A
version of Emacs 30 from early October 2023 still works correctly here.
A version from November 9th exhibits the new behavior.  I can try to
bisect from there but this takes much time on my slow machines.

martin

bug#71774: 31.0.50; Hash table weakness broken?

[Mattias Engdegård]

> Is it possible the problem is fixed by running (setq values nil) before (garbage-collect), or after it, or in between calls to (garbage-collect)? I've not had time to test on current emacs -Q, but on my running emacs session that appears to fix the problem.

Thank you, Pip! It's probably `values` indeed. The effect can also be circumvented by not returning the undead object to the REPL:

  (setq h (make-hash-table :weakness 'value))
  -> #s(hash-table weakness value)

  (progn (puthash t (list 3) h) nil)
  -> nil

  h
  -> #s(hash-table weakness value data (t (3)))

  (garbage-collect)

  h
  #s(hash-table weakness value)

> Of course I have no idea why it worked differently in older emacs versions.

It appears to be an effect of Stefan's change 84f72f19, possibly unintended.
Maybe we should take this opportunity to get rid of `values` altogether, or at least in `eval-last-sexp`.

bug#71774: 31.0.50; Hash table weakness broken?

[Pip Cet via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Tuesday, June 25th, 2024 at 13:57, martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> wrote:
> With Emacs -Q evaluate the following forms in scratch step by step:
> 
> 
> (defvar table (make-hash-table :weakness t))
> 
> (puthash t (split-window) table)
> 
> (delete-other-windows)
> 
> (garbage-collect)
> 
> (gethash t table)

Is it possible the problem is fixed by running (setq values nil) before (garbage-collect), or after it, or in between calls to (garbage-collect)? I've not had time to test on current emacs -Q, but on my running emacs session that appears to fix the problem.

Of course I have no idea why it worked differently in older emacs versions.

Pip

bug#71774: 31.0.50; Hash table weakness broken?

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > Is it possible the problem is fixed by running (setq values nil)

Which "values" would you set to nil?

 > before (garbage-collect), or after it, or in between calls to
 > (garbage-collect)? I've not had time to test on current emacs -Q, but
 > on my running emacs session that appears to fix the problem.

What precisely did you do?

martin

bug#71774: 31.0.50; Hash table weakness broken?

[Pip Cet via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Wednesday, June 26th, 2024 at 08:37, martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> wrote:

> > Is it possible the problem is fixed by running (setq values nil)
> Which "values" would you set to nil?

The global variable "values", which the REPL uses to store evaluated values.

> What precisely did you do?

Here's my *scratch* buffer in emacs -Q:


(defvar table (make-hash-table :weakness t))

(puthash t (split-window) table)

(delete-other-windows)

(garbage-collect)

(gethash t table)

(setq values nil)

(garbage-collect)

(gethash t table)


What I did is go through this buffer line by line, using C-x C-e after each line. The first gethash produces the window, the second returns nil.

Pip

bug#71774: 31.0.50; Hash table weakness broken?

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 >> Which "values" would you set to nil?
 >
 > The global variable "values", which the REPL uses to store evaluated values.
 >
 >> What precisely did you do?
 >
 > Here's my *scratch* buffer in emacs -Q:
 >
 >
 > (defvar table (make-hash-table :weakness t))
 >
 > (puthash t (split-window) table)
 >
 > (delete-other-windows)
 >
 > (garbage-collect)
 >
 > (gethash t table)
 >
 > (setq values nil)
 >
 > (garbage-collect)
 >
 > (gethash t table)
 >
 >
 > What I did is go through this buffer line by line, using C-x C-e after each line. The first gethash produces the window, the second returns nil.

You're right.  So here the scenario

(defvar table (make-hash-table :weakness t))

(progn
   (puthash t (split-window) table)
   (delete-other-windows))

(garbage-collect)

(gethash t table)

gets me nil too.  So to make such tests work one has to tinker with
that obsolete variable 'values'.  Worrisome.

Closing this bug and many thanks, martin

bug#71774: 31.0.50; Hash table weakness broken?

[Pip Cet via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Wednesday, June 26th, 2024 at 09:08, martin rudalics <rudalics@gmx.at> wrote:

> gets me nil too. So to make such tests work one has to tinker with
> that obsolete variable 'values'. Worrisome.

I believe that change, in commit 84f72f19e514db8f8f6e469340fb5fa0719d40b6, might have been unintentional. Stefan: is it intentional that the default behavior was changed to modify "values", even if eval-expression-debug-on-error is `t`?

Pip

bug#71774: 31.0.50; Hash table weakness broken?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> gets me nil too. So to make such tests work one has to tinker with
>> that obsolete variable 'values'. Worrisome.
> I believe that change, in commit 84f72f19e514db8f8f6e469340fb5fa0719d40b6,
> might have been unintentional. Stefan: is it intentional that the default
> behavior was changed to modify "values", even if
> eval-expression-debug-on-error is `t`?

No, indeed, it was an oversight.
The patch below should get us back to the previous behavior, tho I can't
see why we'd set `values` when `eval-expression-debug-on-error` is nil
and not when it's not.

Martin, can you confirm that it fixes the problem for you (so I can
install it on `emacs-30`)?

Apparently this was introduced by

    commit 4428c27c1ae7d5fe5233e8d7b001a8cd2fcdc56f
    Author: Lars Ingebrigtsen <larsi@gnus.org>
    Date:   Mon Feb 8 08:15:45 2021 +0100
    
        Record the value of `C-x C-e' in `values'
        
        * lisp/progmodes/elisp-mode.el (eval-last-sexp): Record the value
        in `values' (bug#22066) since we're messaging it.

I'd be in favor of just removing the call to `values--store-value` as
suggested by Matthias.


        Stefan


diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 7d0312eb2a4..3423b9493f0 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1633,7 +1633,8 @@ eval-last-sexp
 If `eval-expression-debug-on-error' is non-nil, which is the default,
 this command arranges for all errors to enter the debugger."
   (interactive "P")
-  (values--store-value
+  (funcall
+   (if eval-expression-debug-on-error #'values--store-value #'identity)
    (handler-bind ((error (if eval-expression-debug-on-error
                              #'eval-expression--debug #'ignore)))
      (elisp--eval-last-sexp eval-last-sexp-arg-internal))))

bug#71774: 31.0.50; Hash table weakness broken?

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > Martin, can you confirm that it fixes the problem for you (so I can
 > install it on `emacs-30`)?

Unless I did something wrong it does not fix the problem for me.

 > I'd be in favor of just removing the call to `values--store-value` as
 > suggested by Matthias.

Removing that call fixes the problem for me.

martin

bug#71774: 31.0.50; Hash table weakness broken?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Martin, can you confirm that it fixes the problem for you (so I can
>> install it on `emacs-30`)?
> Unless I did something wrong it does not fix the problem for me.

Duh, I reversed the condition.
Can you (re)try with the patch below?


        Stefan


diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 7d0312eb2a4..4aa9d9714cd 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1633,7 +1633,8 @@ eval-last-sexp
 If `eval-expression-debug-on-error' is non-nil, which is the default,
 this command arranges for all errors to enter the debugger."
   (interactive "P")
-  (values--store-value
+  (funcall
+   (if eval-expression-debug-on-error #'identity #'values--store-value)
    (handler-bind ((error (if eval-expression-debug-on-error
                              #'eval-expression--debug #'ignore)))
      (elisp--eval-last-sexp eval-last-sexp-arg-internal))))

bug#71774: 31.0.50; Hash table weakness broken?

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Unless I did something wrong it does not fix the problem for me.
>
> Duh, I reversed the condition.
> Can you (re)try with the patch below?

Works now.

Thanks, martin

bug#71774: 31.0.50; Hash table weakness broken?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> Unless I did something wrong it does not fix the problem for me.
>> Duh, I reversed the condition.
>> Can you (re)try with the patch below?
> Works now.

Thanks, pushed to `emacs-30`, closing.
BTW, `values` has been declared obsolete in etc/NEWS.28 but isn't marked
as obsolete anywhere else.
Should we `make-obsolete-variable` it (on `master`)?


        Stefan

bug#71774: 31.0.50; Hash table weakness broken?

[Stefan Kangas]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> Thanks, pushed to `emacs-30`, closing.
> BTW, `values` has been declared obsolete in etc/NEWS.28 but isn't marked
> as obsolete anywhere else.
> Should we `make-obsolete-variable` it (on `master`)?

Can we do that now?

commit 627a02467508140d213a68c9eed6cb78a5e94860
Author: Lars Ingebrigtsen <larsi@gnus.org>
Date:   Tue Feb 9 16:28:30 2021 +0100

    Note that the `values' variable is now obsolete

    * src/lread.c (syms_of_lread): Note that it's obsolete in the doc
    string (because we can't mark it as obsolete "properly" yet,
    because that leads to compilation warnings when somebody
    (let (values) ... values).

bug#71774: 31.0.50; Hash table weakness broken?

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Can we do that now?

I believe we can, yes 🙂


        Stefan


> commit 627a02467508140d213a68c9eed6cb78a5e94860
> Author: Lars Ingebrigtsen <larsi@gnus.org>
> Date:   Tue Feb 9 16:28:30 2021 +0100
>
>     Note that the `values' variable is now obsolete
>
>     * src/lread.c (syms_of_lread): Note that it's obsolete in the doc
>     string (because we can't mark it as obsolete "properly" yet,
>     because that leads to compilation warnings when somebody
>     (let (values) ... values).

bug#71774: 31.0.50; Hash table weakness broken?

[Eli Zaretskii]

> Cc: Pip Cet <pipcet@protonmail.com>, 71774-done@debbugs.gnu.org,
>  Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 27 Jun 2024 14:42:45 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> >>> Unless I did something wrong it does not fix the problem for me.
> >> Duh, I reversed the condition.
> >> Can you (re)try with the patch below?
> > Works now.
> 
> Thanks, pushed to `emacs-30`, closing.
> BTW, `values` has been declared obsolete in etc/NEWS.28 but isn't marked
> as obsolete anywhere else.
> Should we `make-obsolete-variable` it (on `master`)?

Yes, on master, please.

bug#71774: 31.0.50; Hash table weakness broken?

[Pip Cet via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Wednesday, June 26th, 2024 at 13:23, Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> wrote:

> diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el index 7d0312eb2a4..3423b9493f0 100644 --- a/lisp/progmodes/elisp-mode.el +++ b/lisp/progmodes/elisp-mode.el @@ -1633,7 +1633,8 @@ eval-last-sexp If`eval-expression-debug-on-error' is non-nil, which is the default,
> this command arranges for all errors to enter the debugger."
> (interactive "P")
> - (values--store-value
> + (funcall
> + (if eval-expression-debug-on-error #'values--store-value #'identity)

I think that condition's reversed?

Pip