Re: frames vs. weak hash tables and garbage collection

[Stefan Monnier]

>> The reference from `values' is just due to the fact that you run
>> `reproduce-bug' from M-: and that its return value contains the frame, so
>> it's a rather unusual circumstance.

> I'm confused.  The frame can only show up in values if it fails to be
> garbage collected for another reason.

Right.  In your case the reason is the recent events array.

> Anyway, although I can see that the recent events array would be
> enough to cause the problem, I'm not sure it is the only cause.

Could be, but at least it matched the behavior I saw:
if I hit a key 300 times, set `values' to nil, and call garbage-collect then
the hash-table entry gets deleted (I modified your test case to make the
hash-table global).

>   (defun reproduce-bug ()
>     (let ((ht (make-hash-table :weakness 'key)))
>       (let ((x
>              (make-frame)
>              ;;(get-buffer-create "xyzzy")
>              ))
>         (puthash x t ht)
>         (delete-frame x)
>         ;;(kill-buffer x)
>         )
>       ;; Give time for various frame related events to be handled, in
>       ;; case this is needed.
>       (sit-for 1)
>       ;; There may be a reference to the frame in the array of recent
>       ;; events, so we clear this array.
>       (clear-this-command-keys)
>       ;; In theory, the only reference to the new frame is now the key
>       ;; in the hash table.  Because of the weakness, this key should
>       ;; not keep the frame alive.
>       (garbage-collect)
>       ;; The hash table should now be empty.
>       (let (l)
>         (maphash (lambda (k v) (push (cons k v) l)) ht)
>         l)))

No idea about this one, but it may very well be due to transient effects
such as the fact that the stack is scanned conservatively, so there may
still be a spurious pointer to the frame left over somewhere at the time you
call `garbage-collect'.

Also the sit-for is enough to cause redisplay and execution of process
filters, but I'm not convinced it's enough to cause the frame events to be
added to the "recent events array", so maybe these will appear after the
call to clear-this-command-keys.

>> But it seems that `values' is never flushed, so it is a source of leaks.
>> We should probably fix it (I'd remove it since almost noone even knows that
>> it exists, but I guess Richard uses it once in a blue moon).

> Perhaps it should have a length limit at least?

Yes.  I.e. it should be a ring.


        Stefan