If you were going to take a very conservative approach...

On Sep 28, 2004, at 11:04 AM, Daniel Berger wrote:

> Hi all,
>
> Ruby 1.8.2 (2004-09-22)
> Solaris 9
> gcc 3.3.3
>
> I'm trying to build an extension for kstat on Solaris.  Sample test
> scripts work fine, but test unit blows up with a bus error.  That
> usually means I've got a problem somewhere in my C code with regards
> to memory allocation.  Only, I don't see where.
>
> Here's the .c, .h, extconf.rb, test.rb, and tc_kstat.rb files,
> followed by the backtrace:
>
> /* rkstat.c */
> #include <kstat.h>
> #include <sys/sysinfo.h>
> #include <sys/inttypes.h>
> #include "ruby.h"
> #include "rkstat.h"
>
> VALUE cKstatError;
>
> static VALUE ks_allocate(VALUE klass){
>    KstatStruct* ptr = malloc(sizeof(KstatStruct));
>    return Data_Wrap_Struct(klass,0,ks_free,ptr);
> }
>
> VALUE ks_init(VALUE self){
>    KstatStruct* ptr;
>
>    Data_Get_Struct(self,KstatStruct,ptr);
>
>    ptr->kc = kstat_open();
>    if(NULL == ptr->kc){
>       rb_raise(cKstatError,"kstat_open() failure");
>    }
>
>    ptr->ksp = kstat_lookup(
>       ptr->kc,
>       NULL,
>       -1,
>       NULL
>    );
>
>    if(NULL == ptr->ksp){
>       rb_raise(cKstatError,"kstat_lookup() failure");
>    }
>
>    return self;
> }
>
> VALUE ks_record(VALUE self){
>    VALUE rbMHash, rbIHash, rbNHash, rbSHash;

Shouldn't all these hashes be volatile?
<snip>
>
> static VALUE map_io_data_type(kstat_io_t k){
>    VALUE rbHash = rb_hash_new();
>

volatile rbHash?

>    rb_hash_aset(rbHash,rb_str_new2("nread"),ULL2NUM(k.nread));
>    rb_hash_aset(rbHash,rb_str_new2("nwritten"),ULL2NUM(k.nwritten));
>    rb_hash_aset(rbHash,rb_str_new2("reads"),UINT2NUM(k.reads));
>    rb_hash_aset(rbHash,rb_str_new2("writes"),UINT2NUM(k.writes));
>    rb_hash_aset(rbHash,rb_str_new2("wtime"),ULL2NUM(k.wtime));
>    rb_hash_aset(rbHash,rb_str_new2("wlentime"),ULL2NUM(k.wlentime));
>     
> rb_hash_aset(rbHash,rb_str_new2("wlastupdate"),ULL2NUM(k.wlastupdate));
>    rb_hash_aset(rbHash,rb_str_new2("rtime"),ULL2NUM(k.rtime));
>    rb_hash_aset(rbHash,rb_str_new2("rlentime"),ULL2NUM(k.rlentime));
>     
> rb_hash_aset(rbHash,rb_str_new2("rlastupdate"),ULL2NUM(k.rlastupdate));
>    rb_hash_aset(rbHash,rb_str_new2("wcnt"),UINT2NUM(k.wcnt));
>    rb_hash_aset(rbHash,rb_str_new2("rcnt"),UINT2NUM(k.rcnt));
>

Also, ULL2NUM() (and all the 2NUM() macros) can call rb_gc() if the  
argument is out of Fixnum range (a Bignum will be created - possibly  
calling rb_gc() in the process).  This could cause the key argument to  
be collected by the gc (I think).

>    return rbHash;
> }
>
> static VALUE map_named_data_type(kstat_t* ksp){
>    VALUE rbHash;

volatile rbHash?

>    kstat_named_t* knp;
>    knp = (kstat_named_t *)ksp->ks_data;
>    int i;
>
>    rbHash = rb_hash_new();
>
>    for(i = 0; i < ksp->ks_ndata; i++, knp++)
>    {
>       switch (knp->data_type)
>       {
>          case KSTAT_DATA_CHAR:
>              
> rb_hash_aset(rbHash,rb_str_new2(knp->name),rb_str_new2(knp->value.c));
>             break;
>          case KSTAT_DATA_INT32:
>              
> rb_hash_aset(rbHash,rb_str_new2(knp->name),INT2NUM(knp->value.i32));
>             break;
>          case KSTAT_DATA_UINT32:
>              
> rb_hash_aset(rbHash,rb_str_new2(knp->name),UINT2NUM(knp->value.ui32));
>             break;
>          case KSTAT_DATA_INT64:
>              
> rb_hash_aset(rbHash,rb_str_new2(knp->name),LL2NUM(knp->value.i64));
>             break;
>          case KSTAT_DATA_UINT64:
>              
> rb_hash_aset(rbHash,rb_str_new2(knp->name),ULL2NUM(knp->value.ui64));
>             break;
>          default:
>              
> rb_hash_aset(rbHash,rb_str_new2(knp->name),rb_str_new2("Unknown"));
>             break;

rb_str_new2() can call gc() (more below with backtrace).  In the first  
case and the last case of this switch statement rb_str_new2() is called  
twice, BEFORE rb_hash_aset() is called.  If the second call to  
rb_str_new2() in these cases calls the rb_gc() you have problems.
You could use a function like:

/* untested code */
void
hash_add_pair(VALUE hash, const char *key, const char *value)
{
	volatile VALUE key_obj = rb_str_new2(key);
	rb_hash_aset(hash, key_obj, rb_str_new2(value));
}


>       }
>    }
>    return rbHash;
> }
>
<snip>
> (gdb) backtrace
> #0  0xff31d3d4 in _libc_kill () from /usr/lib/libc.so.1
> #1  0xff2b5698 in abort () from /usr/lib/libc.so.1
> #2  0x000ed618 in rb_bug ()
> #3  0x000b5e50 in sigbus ()
> #4  <signal handler called>
> #5  0x000bad34 in st_foreach ()
> #6  0x00046490 in mark_tbl ()
> #7  0x00046a68 in gc_mark_children ()
> #8  0x000466ec in gc_mark ()
> #9  0x000464f0 in mark_keyvalue ()
> #10 0x000bada8 in st_foreach ()
> #11 0x00046544 in mark_hash ()
> #12 0x00046b6c in gc_mark_children ()
> #13 0x000466ec in gc_mark ()
> #14 0x000463c4 in mark_locations_array ()
> #15 0x00046410 in rb_gc_mark_locations ()
> #16 0x000482dc in rb_gc ()
> #17 0x00045dbc in rb_newobj ()

gc is being called by rb_str_new2() in map_named_data_type()
seems like the most likely place for an error because of the back to  
back calls of rb_str_new2()

> #18 0x000bb068 in str_alloc ()
> #19 0x000bb148 in str_new ()
> #20 0x000bb1f8 in rb_str_new ()
> #21 0x000bb25c in rb_str_new2 ()


> #22 0xff261294 in map_named_data_type () from ./solaris/kstat.so
> #23 0xff261594 in ks_record () from ./solaris/kstat.so
>
<snip>

Disclaimer: these are suggestions... I am by no means an expert.
-Charlie