bug#9900: Using a guardian on a value in a weak hash

bug#9900: Using a guardian on a value in a weak hash

[Ian Price]

Hi guilers,

If I 'guard' a value, and store it in a weak-key hashtable, then it
doesn't appear in the guardian even after it is removed from the
weak-hash by a garbage collection. Note, this only happens in a
_script_, and will work fine in a REPL (you should only need two GCs,
one for the weak hash, and one for the now free value).
e.g.


(define guardian (make-guardian))
(define finalizer-table (make-weak-key-hash-table))

(let ((f (lambda () (display "test\n"))))
  (guardian f)
  (hashq-set! finalizer-table (cons #f #f) f)
  #f)

(write finalizer-table)
(newline)

(gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)

(write finalizer-table)
(newline)

(write (guardian))
(newline)


will produce the output


[ian@Kagami guile]$ guile -s /tmp/gcbug.scm 
#<weak−key−hash−table 1/31>
#<weak−key−hash−table 0/31>
#f
[ian@Kagami guile]$ 

-- 
Ian Price

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"

bug#9900: Using a guardian on a value in a weak hash

[Stefan Israelsson Tampe]

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

f below in the code is referencing a direct function and not a closure that
is not gc:able it seams
although there is no references to it. It looks like the loaded file
references that code and it will not be
gc:ed until the same file is loaded again (make sure to save the old
guardian so that one can inspect)
The weak key hastable is wrongly used and swaping key and value gives
correct behavior

Regards
Stefan

On Fri, Oct 28, 2011 at 10:42 PM, Ian Price <ianprice90@googlemail.com>wrote:

>
> Hi guilers,
>
> If I 'guard' a value, and store it in a weak-key hashtable, then it
> doesn't appear in the guardian even after it is removed from the
> weak-hash by a garbage collection. Note, this only happens in a
> _script_, and will work fine in a REPL (you should only need two GCs,
> one for the weak hash, and one for the now free value).
> e.g.
>
>
> (define guardian (make-guardian))
> (define finalizer-table (make-weak-key-hash-table))
>
> (let ((f (lambda () (display "test\n"))))
>  (guardian f)
>  (hashq-set! finalizer-table (cons #f #f) f)
>  #f)
>
> (write finalizer-table)
> (newline)
>
> (gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)(gc)
>
> (write finalizer-table)
> (newline)
>
> (write (guardian))
> (newline)
>
>
> will produce the output
>
>
> [ian@Kagami guile]$ guile -s /tmp/gcbug.scm
> #<weak-key-hash-table 1/31>
> #<weak-key-hash-table 0/31>
> #f
> [ian@Kagami guile]$
>
> --
> Ian Price
>
> "Programming is like pinball. The reward for doing it well is
> the opportunity to do it again" - from "The Wizardy Compiled"
>
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 2118 bytes --]

bug#9900: Using a guardian on a value in a weak hash

[Ian Price]

Stefan Israelsson Tampe <stefan.itampe@gmail.com> writes:

> f below in the code is referencing a direct function and not a closure that is not gc:able it seams
> although there is no references to it. It looks like the loaded file references that code and it will not be
> gc:ed until the same file is loaded again (make sure to save the old guardian so that one can inspect)

This does seem to be the case

scheme@(guile−user)> (load "gcbug.scm")
;;; note: source file /tmp/gcbug.scm
;;;       newer than compiled /home/ian/.cache/guile/ccache/2.0−LE−4−2.0/tmp/gcbug.scm.go
;;; note: auto−compilation is enabled, set GUILE_AUTO_COMPILE=0
;;;       or pass the −−no−auto−compile argument to disable.
;;; compiling /tmp/gcbug.scm
;;; compiled /home/ian/.cache/guile/ccache/2.0−LE−4−2.0/tmp/gcbug.scm.go
#<weak−key−hash−table 1/31>
#<weak−key−hash−table 0/31>
#f
scheme@(guile−user)> guardian
$1 = #<guardian 8503710 (reachable: 1 unreachable: 0)>
scheme@(guile−user)> (guardian)
$2 = #f
scheme@(guile−user)> (guardian)
$3 = #f
scheme@(guile−user)> (define k guardian)
scheme@(guile−user)> (load "gcbug.scm")
#<weak−key−hash−table 1/31>
#<weak−key−hash−table 0/31>
#f
scheme@(guile−user)> (guardian)
$4 = #f
scheme@(guile−user)> (guardian)
$5 = #f
scheme@(guile−user)> (k)
$6 = (foo . foo)
scheme@(guile−user)> (k)
$7 = #f
scheme@(guile−user)> 

here, f is the pair (foo . foo).

> The weak key hastable is wrongly used and swaping key and value gives
> correct behavior
I disagree. While I would certainly expect it to work if I made the
reference to f weak, it would miss the point of my code entirely. Namely
to make sure that f isn't gc'd until after some other value is (hence
the weak reference to a cons, and my comment about expecting to need 2 gcs).

FWIW, I swapped it and ran again

[ian@Kagami tmp]$ guile -s gcbug.scm 
;;; note: source file /tmp/gcbug.scm
;;;       newer than compiled /home/ian/.cache/guile/ccache/2.0−LE−4−2.0/tmp/gcbug.scm.go
;;; note: auto−compilation is enabled, set GUILE_AUTO_COMPILE=0
;;;       or pass the −−no−auto−compile argument to disable.
;;; compiling /tmp/gcbug.scm
;;; compiled /home/ian/.cache/guile/ccache/2.0−LE−4−2.0/tmp/gcbug.scm.go
#<weak−key−hash−table 1/31>
#<weak−key−hash−table 1/31>
#f

which isn't correct either, but not surprising if something else is
holding onto f.

-- 
Ian Price

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"

bug#9900: local vars not cleaned

[Stefan Israelsson Tampe]

[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]

>> This does seem to be the case

Yes. I toke a code snippet and compiled to assembly. It looks like the .go
file contain a specification
of a program with a set of local variables where the (cons 'foo 'foo) is
stored very much like the function
local vars on the stack. Now there is a link somehow to this program that
is alive after it has been executed
and itself links to the list of local vars. This theory can be shown by
doing the same thing in a new let
 and say (cons 'goo 'goo) afterwards for which the old local slot is used
as storage. Hence at evaluation
(gauardian) will show (foo . foo) but (goo . goo) will not appear due to
beeing referenced by the local var slot.

Now, one solution would be to have a pass of cleaning the local slots after
the execution because this behavior
has the potential of leading to memory leaks. The next step is to question
the use of keeping the link to the program
that comes from the loaded file, maybe due to cashing this is an effective
strategy, but someone with greater overview
of the code has to commment on this. In alles locally defined functions and
variables should not be kept in the gc because
it is begging to yield memory leaks. cashing locally defined functions is a
bit weaker and on first sight can be a good cashing strategy. On the other
hand if one implementing schemes like Ian does one can easilly have a chain
of objects that has to be gc:d is a specific
order and if one of these object is a local lambda gcing of the elements
further down will not happen.


> I disagree. While I would certainly expect it to work if I made the
> reference to f weak, it would miss the point of my code entirely. Namely
> to make sure that f isn't gc'd until after some other value is (hence
> the weak reference to a cons, and my comment about expecting to need 2
gcs).

Sorry for may blunt statement here. You seam to come from a valid way of
doing your coding. My fault.

/Stefan

[-- Attachment #2: Type: text/html, Size: 2086 bytes --]

bug#9900: local vars not cleaned

[Stefan Israelsson Tampe]

[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

Sorry to spam the list.

But beeing a little ignorant how guile works, makes this a discovering
procedure.

So locals is always allocated from the stack. This is what happens with
Ians code.

A program is executed and a set of locals is allocated on the stack for the
duration of the loading, this means that during the
whole execution of the loaded file the locals variables are below the stack
pointer and hence always contains a reference from the stack to the last
used objects in the local variables. This can be seen by loading the file
and then do an explicit gc on the repl. and then check the guardian. Then
because the sp pointer is now below the locals in the loaded file they can
be gc:ed and is also returned by calls to the guardian.

What can be done?

1. One can push the constructors into functions that are called from the
toplevel in the code
2. One can patch guile so that used locals in let constructs are cleaned at
the end of the let form in
    a) toplevel let
    b) all let

So the question now is if this is going to be fixed or if it is going to be
a subtle point that can trick
advanced users of guile but work most of the time in the name of speed.

what do you think?

/Stefan

[-- Attachment #2: Type: text/html, Size: 1302 bytes --]

bug#9900: local vars not cleaned

[Ludovic Courtès]

Hi Stefan,

Stefan Israelsson Tampe <stefan.itampe@gmail.com> skribis:

> A program is executed and a set of locals is allocated on the stack for the
> duration of the loading, this means that during the
> whole execution of the loaded file the locals variables are below the stack
> pointer and hence always contains a reference from the stack to the last
> used objects in the local variables.

Normally the GC will only scan the relevant part of the VM stack–see
‘VM_ENABLE_PRECISE_STACK_GC_SCAN’ in vm.c.  If you think this is not
working as advertised, can you add a breakpoint in ‘vm_stack_mark’ and
see what happens?

To check your hypothesis you could also define ‘VM_ENABLE_STACK_NULLING’
in vm.c.

TIA! :-)

Ludo’.

bug#9900: local vars not cleaned

[Andy Wingo]

Hi,

On Tue 01 Nov 2011 01:20, ludo@gnu.org (Ludovic Courtès) writes:

> Stefan Israelsson Tampe <stefan.itampe@gmail.com> skribis:
>
>> A program is executed and a set of locals is allocated on the stack for the
>> duration of the loading, this means that during the
>> whole execution of the loaded file the locals variables are below the stack
>> pointer and hence always contains a reference from the stack to the last
>> used objects in the local variables.
>
> Normally the GC will only scan the relevant part of the VM stack–see
> ‘VM_ENABLE_PRECISE_STACK_GC_SCAN’ in vm.c.

I think Stefan's analysis is right.  VM marking is working as
advertised.  The problem is that the slot corresponding to the lexical
`f' has not been nulled out.  We can probably fix this by doing a
local-set of the slot to `undefined' after leaving a `let', `letrec', or
`fix' binding in non-tail context.

Andy
-- 
http://wingolog.org/

bug#9900: Using a guardian on a value in a weak hash

[Andy Wingo]

I believe I have fixed this bug in stable-2.0.  Thanks for the report!

Andy
-- 
http://wingolog.org/

bug#9900: Using a guardian on a value in a weak hash

[Ludovic Courtès]

Hello!

It could be a coincidence, but the “Unused modules are removed” test has
been failing on i686-linux-gnu since commit
fb135e12a473fd9a1612a59f904cfb90877fe775 (according to
<http://hydra.nixos.org/build/1574623>.)

I can’t see how this commit could lead to a reference leak that would
make the test fail, but I wanted to double-check with you.

Ideas?

Thanks,
Ludo’.

bug#9900: Using a guardian on a value in a weak hash

[Andy Wingo]

On Wed 14 Dec 2011 22:49, ludo@gnu.org (Ludovic Courtès) writes:

> Hello!
>
> It could be a coincidence, but the “Unused modules are removed” test has
> been failing on i686-linux-gnu since commit
> fb135e12a473fd9a1612a59f904cfb90877fe775 (according to
> <http://hydra.nixos.org/build/1574623>.)
>
> I can’t see how this commit could lead to a reference leak that would
> make the test fail, but I wanted to double-check with you.
>
> Ideas?

Is the stack-clearing code not doing its job, somehow?  Hummmm.

I'll look into this later if no one gets around to it, but investigation
would be appreciated ;-)

Cheers,

Andy
-- 
http://wingolog.org/

bug#9900: Using a guardian on a value in a weak hash

[Ludovic Courtès]

Hi!

Andy Wingo <wingo@pobox.com> skribis:

> On Wed 14 Dec 2011 22:49, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Hello!
>>
>> It could be a coincidence, but the “Unused modules are removed” test has
>> been failing on i686-linux-gnu since commit
>> fb135e12a473fd9a1612a59f904cfb90877fe775 (according to
>> <http://hydra.nixos.org/build/1574623>.)
>>
>> I can’t see how this commit could lead to a reference leak that would
>> make the test fail, but I wanted to double-check with you.
>>
>> Ideas?
>
> Is the stack-clearing code not doing its job, somehow?  Hummmm.

Well, there was no stack-clearing code before anyway, so I can’t see how
this could be the problem.  Yet, it’s annoying.  ;-)

The failure shows up on i686 and not on x86_64.

Thanks,
Ludo’.