‘set-cdr!’ and weak-cdr pairs

‘set-cdr!’ and weak-cdr pairs

[Ludovic Courtès]

Hello,

A bit of explanation for commit
ca33b501a93f8de389c1e3e1bc987f63b6912029...

Try this:

--8<---------------cut here---------------start------------->8---
(use-modules (srfi srfi-1)
             (srfi srfi-9))

(define-record-type <foo>
  (make-foo x)
  foo?
  (x foo-x))

(define register!
  (let ((t (make-weak-value-hash-table 10)))
    (lambda (x)
      (let ((k+v (hash-create-handle! t x #f)))
        (or (cdr k+v)
            (let ((o (make-foo x)))
              (set-cdr! k+v o)
              o))))))

(every (lambda (x)
         (make-foo #f)
         (let ((o (register! x)))
           (or (foo? o)
               (pk 'bad! o))))
       (circular-list 1 2 3 4 5 6 7 8 9))
--8<---------------cut here---------------end--------------->8---

Eventually ‘(foo? o)’ fails and Guile segfaults while trying to display O.

Changing ‘register!’ to the following works:

--8<---------------cut here---------------start------------->8---
(define register!
  (let ((t (make-weak-value-hash-table 10)))
    (lambda (x)
      (or (hash-ref t x)
          (let ((o (make-foo x)))
            (hash-set! t x o)
            o)))))
--8<---------------cut here---------------end--------------->8---

The problem is that ‘hash-create-handle!’ above created a weak-cdr
pair—i.e., a pair whose cdr is /not/ scanned for pointers—but ‘set-cdr!’
did not register a disappearing link from O to K+V.  Consequently, O
eventually gets collected, but K+V remains; the storage of O then gets
reused, and the cdr of K+V ends up containing either an unrelated or >an
invalid Scheme object.

This problem is explicitly addressed in ‘scm_hash_fn_set_x’.  AFAICS
‘set-cdr!’ has no way of knowing whether its passed a normal pair or a
weak-cdr one, so it cannot be changed to handle weak-cdr pairs
gracefully.

And of course, we have the same problem with weak-car pairs and
‘set-car!’, but ‘set-car!’ is unlikely to be used on weak-car pairs.

Thoughts?

Ludo’.

Re: ‘set-cdr!’ and weak-cdr pairs

[Andy Wingo]

On Sun 13 Mar 2011 16:25, ludo@gnu.org (Ludovic Courtès) writes:

> The problem is that ‘hash-create-handle!’ above created a weak-cdr
> pair—i.e., a pair whose cdr is /not/ scanned for pointers—but ‘set-cdr!’
> did not register a disappearing link from O to K+V.  Consequently, O
> eventually gets collected, but K+V remains; the storage of O then gets
> reused, and the cdr of K+V ends up containing either an unrelated or >an
> invalid Scheme object.

Given that we don't expose weak-pair constructors or accessors to
Scheme, we should not expose weak pairs to Scheme.  What do you think
about making it an error to hash-create-handle! on a weak table?  That
way you never expose a weak pair to Scheme.  It does appear possible to
discriminate in C between calls to create-handle! that occur due to ref
/ set! and those that are called explicitly.

Also that would free up our implementation to use something other than
weak pairs for weak hash tables.  In particular, I think first-class
weak references are not a bad idea, as Chez Scheme does.

WDYT?

Andy
-- 
http://wingolog.org/

Re: ‘set-cdr!’ and weak-cdr pairs

[Ludovic Courtès]

Hello!

Andy Wingo <wingo@pobox.com> writes:

> On Sun 13 Mar 2011 16:25, ludo@gnu.org (Ludovic Courtès) writes:
>
>> The problem is that ‘hash-create-handle!’ above created a weak-cdr
>> pair—i.e., a pair whose cdr is /not/ scanned for pointers—but ‘set-cdr!’
>> did not register a disappearing link from O to K+V.  Consequently, O
>> eventually gets collected, but K+V remains; the storage of O then gets
>> reused, and the cdr of K+V ends up containing either an unrelated or >an
>> invalid Scheme object.
>
> Given that we don't expose weak-pair constructors or accessors to
> Scheme, we should not expose weak pairs to Scheme.  What do you think
> about making it an error to hash-create-handle! on a weak table?  That
> way you never expose a weak pair to Scheme.  It does appear possible to
> discriminate in C between calls to create-handle! that occur due to ref
> / set! and those that are called explicitly.

Yes, sounds good.  Would you like to work on it?  :-)

Thanks,
Ludo’.

Re: ‘set-cdr!’ and weak-cdr pairs

[Andy Wingo]

Hi :)

On Sun 27 Mar 2011 15:29, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> On Sun 13 Mar 2011 16:25, ludo@gnu.org (Ludovic Courtès) writes:
>>
>>> The problem is that ‘hash-create-handle!’ above created a weak-cdr
>>> pair—i.e., a pair whose cdr is /not/ scanned for pointers—but ‘set-cdr!’
>>> did not register a disappearing link from O to K+V.  Consequently, O
>>> eventually gets collected, but K+V remains; the storage of O then gets
>>> reused, and the cdr of K+V ends up containing either an unrelated or >an
>>> invalid Scheme object.
>>
>> Given that we don't expose weak-pair constructors or accessors to
>> Scheme, we should not expose weak pairs to Scheme.  What do you think
>> about making it an error to hash-create-handle! on a weak table?  That
>> way you never expose a weak pair to Scheme.  It does appear possible to
>> discriminate in C between calls to create-handle! that occur due to ref
>> / set! and those that are called explicitly.
>
> Yes, sounds good.  Would you like to work on it?  :-)

I have done this now.  There were a few places within Guile that were
using the get-handle / create-handle! API with weak hash tables, which I
fixed.  It's quite possible that user code also does this, but between
random failures to `(car x)' and preventative errors, I chose the
latter.  We can back it down to a warning if that's the right thing to
do, though.

Andy
-- 
http://wingolog.org/

Re: ‘set-cdr!’ and weak-cdr pairs

[Ludovic Courtès]

Hello!

Andy Wingo <wingo@pobox.com> writes:

> On Sun 27 Mar 2011 15:29, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Andy Wingo <wingo@pobox.com> writes:
>>
>>> On Sun 13 Mar 2011 16:25, ludo@gnu.org (Ludovic Courtès) writes:
>>>
>>>> The problem is that ‘hash-create-handle!’ above created a weak-cdr
>>>> pair—i.e., a pair whose cdr is /not/ scanned for pointers—but ‘set-cdr!’
>>>> did not register a disappearing link from O to K+V.  Consequently, O
>>>> eventually gets collected, but K+V remains; the storage of O then gets
>>>> reused, and the cdr of K+V ends up containing either an unrelated or >an
>>>> invalid Scheme object.
>>>
>>> Given that we don't expose weak-pair constructors or accessors to
>>> Scheme, we should not expose weak pairs to Scheme.  What do you think
>>> about making it an error to hash-create-handle! on a weak table?  That
>>> way you never expose a weak pair to Scheme.  It does appear possible to
>>> discriminate in C between calls to create-handle! that occur due to ref
>>> / set! and those that are called explicitly.
>>
>> Yes, sounds good.  Would you like to work on it?  :-)
>
> I have done this now.  There were a few places within Guile that were
> using the get-handle / create-handle! API with weak hash tables, which I
> fixed.  It's quite possible that user code also does this, but between
> random failures to `(car x)' and preventative errors, I chose the
> latter.  We can back it down to a warning if that's the right thing to
> do, though.

Cool, thanks for working on it!  I skimmed over the relevant patches and
it looks good to me.

Ludo’.