* ‘set-cdr!’ and weak-cdr pairs
@ 2011-03-13 15:25 Ludovic Courtès
2011-03-27 11:22 ` Andy Wingo
0 siblings, 1 reply; 5+ messages in thread
From: Ludovic Courtès @ 2011-03-13 15:25 UTC (permalink / raw)
To: guile-devel
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’.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ‘set-cdr!’ and weak-cdr pairs
2011-03-13 15:25 ‘set-cdr!’ and weak-cdr pairs Ludovic Courtès
@ 2011-03-27 11:22 ` Andy Wingo
2011-03-27 13:29 ` Ludovic Courtès
0 siblings, 1 reply; 5+ messages in thread
From: Andy Wingo @ 2011-03-27 11:22 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guile-devel
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/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ‘set-cdr!’ and weak-cdr pairs
2011-03-27 11:22 ` Andy Wingo
@ 2011-03-27 13:29 ` Ludovic Courtès
2011-05-01 21:14 ` Andy Wingo
0 siblings, 1 reply; 5+ messages in thread
From: Ludovic Courtès @ 2011-03-27 13:29 UTC (permalink / raw)
To: guile-devel
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’.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ‘set-cdr!’ and weak-cdr pairs
2011-03-27 13:29 ` Ludovic Courtès
@ 2011-05-01 21:14 ` Andy Wingo
2011-05-05 18:20 ` Ludovic Courtès
0 siblings, 1 reply; 5+ messages in thread
From: Andy Wingo @ 2011-05-01 21:14 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guile-devel
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/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ‘set-cdr!’ and weak-cdr pairs
2011-05-01 21:14 ` Andy Wingo
@ 2011-05-05 18:20 ` Ludovic Courtès
0 siblings, 0 replies; 5+ messages in thread
From: Ludovic Courtès @ 2011-05-05 18:20 UTC (permalink / raw)
To: guile-devel
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’.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-05-05 18:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-13 15:25 ‘set-cdr!’ and weak-cdr pairs Ludovic Courtès
2011-03-27 11:22 ` Andy Wingo
2011-03-27 13:29 ` Ludovic Courtès
2011-05-01 21:14 ` Andy Wingo
2011-05-05 18:20 ` Ludovic Courtès
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).