Re: possible gc/weak hash table bug

Re: possible gc/weak hash table bug

[michaelawells]

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

> On Fri 18 Mar 2011 12:51, address@hidden (Ludovic CourtÃs) writes:
> 
> > michaelawells <address@hidden> writes:
> >
> >>>   size_t len = SCM_HASHTABLE_N_ITEMS (table);
> >>> 
> >>>   while (k--)
> >>>     {
> >>>       size_t removed;
> >>>       SCM alist = SCM_SIMPLE_VECTOR_REF (buckets, k);
> >>>       alist = scm_fixup_weak_alist (alist, &removed);  <<<**** FAILS HERE
> >>>       assert (removed <= len);
> >
> > Andy, isnât this assertion bogus since N_ITEMS is updated lazily and
> > thus may not correspond to the current number of items?
> 
> I don't think so; this is the procedure that is doing that lazy sweep.
> 
> Michael, how are you using this hash table?  Are you accessing it from
> different pthreads?  What version of libgc are you using?  (Did you
> enable parallel collection?)
Sorry, the "<<<*** FAILS here" note should have been placed after the assert, not the call to "scm_fixup_weak_alist".

I'm not using weak hash tables in my code.   Weak hash tables are used in modules such as (ice-9 boot-9) and (ice-9 popen), and from C code, such as libguile/symbols.c.  

I'm using gc-7.1.  I didn't use any special options when building libgc.  I'll try rebuilding libgc using the "--enable-threads=posix" configure option and see if I still see the problem.  (Is that what you meant by enabling parallel collection?  If so, perhaps the README file should updated to say this is necessary.)

I'm not using guile from multiple threads, but I am running guile from a thread in a multi-threaded program.  (I've been using guile since before it had support for multiple threads.  So, I constructed the test harness so that guile is called from a single thread.)

Thanks,

Michael Wells


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

Re: possible gc/weak hash table bug

[michaelawells]

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

I rebuilt the gc-7.1 library, this time using the "--enable-threads=posix" configure option.  The exact same library was built.  It would appear that configure, at least on my target, enables threads by default.

On Mar 25, 2011, at 4:48 PM, michaelawells wrote:

>> On Fri 18 Mar 2011 12:51, address@hidden (Ludovic CourtÃs) writes:
>> 
>> > michaelawells <address@hidden> writes:
>> >
>> >>>   size_t len = SCM_HASHTABLE_N_ITEMS (table);
>> >>> 
>> >>>   while (k--)
>> >>>     {
>> >>>       size_t removed;
>> >>>       SCM alist = SCM_SIMPLE_VECTOR_REF (buckets, k);
>> >>>       alist = scm_fixup_weak_alist (alist, &removed);  <<<**** FAILS HERE
>> >>>       assert (removed <= len);
>> >
>> > Andy, isnât this assertion bogus since N_ITEMS is updated lazily and
>> > thus may not correspond to the current number of items?
>> 
>> I don't think so; this is the procedure that is doing that lazy sweep.
>> 
>> Michael, how are you using this hash table?  Are you accessing it from
>> different pthreads?  What version of libgc are you using?  (Did you
>> enable parallel collection?)
> Sorry, the "<<<*** FAILS here" note should have been placed after the assert, not the call to "scm_fixup_weak_alist".
> 
> I'm not using weak hash tables in my code.   Weak hash tables are used in modules such as (ice-9 boot-9) and (ice-9 popen), and from C code, such as libguile/symbols.c.  
> 
> I'm using gc-7.1.  I didn't use any special options when building libgc.  I'll try rebuilding libgc using the "--enable-threads=posix" configure option and see if I still see the problem.  (Is that what you meant by enabling parallel collection?  If so, perhaps the README file should updated to say this is necessary.)
> 
> I'm not using guile from multiple threads, but I am running guile from a thread in a multi-threaded program.  (I've been using guile since before it had support for multiple threads.  So, I constructed the test harness so that guile is called from a single thread.)
> 
> Thanks,
> 
> Michael Wells
> 


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

Re: possible gc/weak hash table bug

[Andy Wingo]

Hi Michael,

On Fri 25 Mar 2011 22:48, michaelawells <michaelawells@gmail.com> writes:

>     On Fri 18 Mar 2011 12:51, address@hidden (Ludovic CourtÃs) writes:
>     
>     > michaelawells <address@hidden> writes:
>     >
>     >>>   size_t len = SCM_HASHTABLE_N_ITEMS (table);
>     >>> 
>     >>>   while (k--)
>     >>>     {
>     >>>       size_t removed;
>     >>>       SCM alist = SCM_SIMPLE_VECTOR_REF (buckets, k);
>     >>>       alist = scm_fixup_weak_alist (alist, &removed);  <<<**** FAILS HERE
>     >>>       assert (removed <= len);
>     >
>     Michael, how are you using this hash table?  Are you accessing it from
>     different pthreads?  What version of libgc are you using?  (Did you
>     enable parallel collection?)
>     
> I'm not using weak hash tables in my code.  Weak hash tables are used
> in modules such as (ice-9 boot-9) and (ice-9 popen), and from C code,
> such as libguile/symbols.c.
>
> I'm using gc-7.1.  I didn't use any special options when building
> libgc.  I'll try rebuilding libgc using the "--enable-threads=posix"
> configure option and see if I still see the problem.  (Is that what
> you meant by enabling parallel collection?  If so, perhaps the README
> file should updated to say this is necessary.)

I referred to an option of libgc called "parallel marking", which you
have to enable when you build libgc.  I'm assuming you did not enable
it.  (What platform are you on?  I suspect that we have not tested all
of the possible gentoo configurations, for example.)  You can find this
out by calling GC_get_parallel() once guile is initialized.

The --enable-threads=posix is indeed the default, and makes Guile
threadsafe and adds support for threads within Guile.

> I'm not using guile from multiple threads, but I am running guile from
> a thread in a multi-threaded program.  (I've been using guile since
> before it had support for multiple threads.  So, I constructed the
> test harness so that guile is called from a single thread.)

OK, this is good.  Can you test again with 2.0.1?  

Cheers,

Andy
-- 
http://wingolog.org/

Re: possible gc/weak hash table bug

[michaelawells]

As requested, I'm posting this to bug-guile@gnu.org.

I'm now seeing this assertion failure, using a guile development snapshot (guile-2.0.0.104-f5fc7):

> dpm.linux: hashtab.c:136: vacuum_weak_hash_table: Assertion `removed <= len' failed.


> static void
> vacuum_weak_hash_table (SCM table)
> {
>   SCM buckets = SCM_HASHTABLE_VECTOR (table);
>   unsigned long k = SCM_SIMPLE_VECTOR_LENGTH (buckets);
>   size_t len = SCM_HASHTABLE_N_ITEMS (table);
> 
>   while (k--)
>     {
>       size_t removed;
>       SCM alist = SCM_SIMPLE_VECTOR_REF (buckets, k);
>       alist = scm_fixup_weak_alist (alist, &removed);  <<<**** FAILS HERE
>       assert (removed <= len);
>       len -= removed;
>       SCM_SIMPLE_VECTOR_SET (buckets, k, alist);
>     }
> 
>   SCM_SET_HASHTABLE_N_ITEMS (table, len);
> }
> 


Unfortunately, this problem is infrequent and intermittent.  In my case, it happened in 10 out of 1000 runs.  

Currently, I don't know of a way to reproduce the problem.



On Mar 17, 2011, at 4:43 AM, Andy Wingo wrote:

> Hi Michael,
> 
> Thanks for the report.  In the future, please copy bug-guile@gnu.org; it
> doesn't hurt.
> 
> On Thu 17 Mar 2011 00:56, michaelawells <michaelawells@gmail.com> writes:
> 
>> hashtab.c:203: weak_bucket_assoc: Assertion `(((scm_t_hashtable *)
>> ((((scm_t_bits) (0? (*(SCM*)0=((((SCM *)((scm_t_cell *) (((scm_t_bits)
>> (0? (*(SCM*)0=((((table))))): (((table)))))))) [((2))]))): (((SCM
>> *)((scm_t_cell *) (((scm_t_bits) (0? (*(SCM*)0=((((table))))):
>> (((table)))))))) [((2))]))))))->n_items) >= args.removed_items'
>> failed.
>> 
>> Did you ever see this assertion fail?  
> 
> No I didn't, and that bothers me a little.
> 
>> I tried using a development snapshot (guile-2.0.0.104-f5fc7).  When I
>> use this snapshot, I do not see this problem.
> 
> There were certainly some bugs fixed.  Let's assume that it's fixed in
> git, and in the upcoming 2.0.1 (should be a week or so).  There are
> still some lingering issues with weak hash tables, but we are working on
> them; and certainly no crashes that we know of.
> 
> Have fun with Guile,
> 
> Andy
> -- 
> http://wingolog.org/

Re: possible gc/weak hash table bug

[michaelawells]

I replaced vacuum_weak_hash_table with the following:

> static void
> vacuum_weak_hash_table (SCM table)
> {
>   SCM buckets = SCM_HASHTABLE_VECTOR (table);
>   unsigned long k = SCM_SIMPLE_VECTOR_LENGTH (buckets);
>   size_t len = SCM_HASHTABLE_N_ITEMS (table);
> 
>   unsigned long org_k = SCM_SIMPLE_VECTOR_LENGTH (buckets);
>   size_t org_len = SCM_HASHTABLE_N_ITEMS (table);
> 
> 
>   if (len>0) {
>   while (k--)
>     {
>       size_t removed;
>       SCM alist = SCM_SIMPLE_VECTOR_REF (buckets, k);
>       alist = scm_fixup_weak_alist (alist, &removed);
>       if (! (removed <= len)) {
> 	fprintf(stderr,"$$$ k = %lu (%lu) removed = %i   len = %i (%i)\n",k,org_k,removed,len,org_len);
>       }
>       assert (removed <= len);
>       len -= removed;
>       SCM_SIMPLE_VECTOR_SET (buckets, k, alist);
>     }
>   
>   SCM_SET_HASHTABLE_N_ITEMS (table, len);
>   }
> }

Two failure occurred in two different runs.  Each time, the debugging message was the same:

> $$$ k = 89 (113) removed = 1   len = 0 (5)
> hashtab.c:145: vacuum_weak_hash_table: Assertion `removed <= len' failed.

This happened while vacuuming bucket 89 (of 113).  The original length was 5.

Again, this problem occurs both infrequently and intermittently.  

On Mar 17, 2011, at 7:09 PM, michaelawells wrote:

> As requested, I'm posting this to bug-guile@gnu.org.
> 
> I'm now seeing this assertion failure, using a guile development snapshot (guile-2.0.0.104-f5fc7):
> 
>> dpm.linux: hashtab.c:136: vacuum_weak_hash_table: Assertion `removed <= len' failed.
> 
> 
>> static void
>> vacuum_weak_hash_table (SCM table)
>> {
>>  SCM buckets = SCM_HASHTABLE_VECTOR (table);
>>  unsigned long k = SCM_SIMPLE_VECTOR_LENGTH (buckets);
>>  size_t len = SCM_HASHTABLE_N_ITEMS (table);
>> 
>>  while (k--)
>>    {
>>      size_t removed;
>>      SCM alist = SCM_SIMPLE_VECTOR_REF (buckets, k);
>>      alist = scm_fixup_weak_alist (alist, &removed);  <<<**** FAILS HERE
>>      assert (removed <= len);
>>      len -= removed;
>>      SCM_SIMPLE_VECTOR_SET (buckets, k, alist);
>>    }
>> 
>>  SCM_SET_HASHTABLE_N_ITEMS (table, len);
>> }
>> 
> 
> 
> Unfortunately, this problem is infrequent and intermittent.  In my case, it happened in 10 out of 1000 runs.  
> 
> Currently, I don't know of a way to reproduce the problem.
> 
> 
> 
> On Mar 17, 2011, at 4:43 AM, Andy Wingo wrote:
> 
>> Hi Michael,
>> 
>> Thanks for the report.  In the future, please copy bug-guile@gnu.org; it
>> doesn't hurt.
>> 
>> On Thu 17 Mar 2011 00:56, michaelawells <michaelawells@gmail.com> writes:
>> 
>>> hashtab.c:203: weak_bucket_assoc: Assertion `(((scm_t_hashtable *)
>>> ((((scm_t_bits) (0? (*(SCM*)0=((((SCM *)((scm_t_cell *) (((scm_t_bits)
>>> (0? (*(SCM*)0=((((table))))): (((table)))))))) [((2))]))): (((SCM
>>> *)((scm_t_cell *) (((scm_t_bits) (0? (*(SCM*)0=((((table))))):
>>> (((table)))))))) [((2))]))))))->n_items) >= args.removed_items'
>>> failed.
>>> 
>>> Did you ever see this assertion fail?  
>> 
>> No I didn't, and that bothers me a little.
>> 
>>> I tried using a development snapshot (guile-2.0.0.104-f5fc7).  When I
>>> use this snapshot, I do not see this problem.
>> 
>> There were certainly some bugs fixed.  Let's assume that it's fixed in
>> git, and in the upcoming 2.0.1 (should be a week or so).  There are
>> still some lingering issues with weak hash tables, but we are working on
>> them; and certainly no crashes that we know of.
>> 
>> Have fun with Guile,
>> 
>> Andy
>> -- 
>> http://wingolog.org/
> 

Re: possible gc/weak hash table bug

[Ludovic Courtès]

Hi,

michaelawells <michaelawells@gmail.com> writes:

>>   size_t len = SCM_HASHTABLE_N_ITEMS (table);
>> 
>>   while (k--)
>>     {
>>       size_t removed;
>>       SCM alist = SCM_SIMPLE_VECTOR_REF (buckets, k);
>>       alist = scm_fixup_weak_alist (alist, &removed);  <<<**** FAILS HERE
>>       assert (removed <= len);

Andy, isn’t this assertion bogus since N_ITEMS is updated lazily and
thus may not correspond to the current number of items?

Thanks,
Ludo’.

Re: possible gc/weak hash table bug

[Andy Wingo]

On Fri 18 Mar 2011 12:51, ludo@gnu.org (Ludovic Courtès) writes:

> michaelawells <michaelawells@gmail.com> writes:
>
>>>   size_t len = SCM_HASHTABLE_N_ITEMS (table);
>>> 
>>>   while (k--)
>>>     {
>>>       size_t removed;
>>>       SCM alist = SCM_SIMPLE_VECTOR_REF (buckets, k);
>>>       alist = scm_fixup_weak_alist (alist, &removed);  <<<**** FAILS HERE
>>>       assert (removed <= len);
>
> Andy, isn’t this assertion bogus since N_ITEMS is updated lazily and
> thus may not correspond to the current number of items?

I don't think so; this is the procedure that is doing that lazy sweep.

Michael, how are you using this hash table?  Are you accessing it from
different pthreads?  What version of libgc are you using?  (Did you
enable parallel collection?)

Andy
-- 
http://wingolog.org/