From: nobu.nokada@... Date: 2004-05-21T11:29:20+09:00 Subject: Re: [PATCH] ruby/dl Hi, At Thu, 20 May 2004 01:07:23 +0900, Jeff Mitchell wrote in [ruby-core:02905]: > ext/dl/ptr.c:26: > > static void > rb_dlmem_delete(void *ptr) > { > rb_secure(4); > rb_hash_delete(DLMemoryTable, DLLONG2NUM(ptr)); > } > > Eventually ptr crosses the boundary of fixnum, causing a bignum to be > created. It appears to be an error to create a non-immediate VALUE > while inside a free() function (memory corruption or segfault). It's not a problem. > Here's my attempted fix. Note that I change the keys of > DLMemoryTable. From what I could decipher from the Japanese > documentation, it doesn't appear that DLMemoryTable is part of the > API. Your modification doesn't work with odd value pointers, though I agree that DLMemoryTable shouldn't be a matter. I guess that the purpose of conversion from addresses and PtrData to Integer is to get rid of marking for GC, then st_table would be better rather than Hash. > There was also a memory leak in dlptr_free(). Indeed, but it should be dlfree(). At Thu, 13 May 2004 04:00:41 +0900, Jeff Mitchell wrote in [ruby-core:02867]: > A simpler yet unorthodox example is: > > require 'dl' > srand(2727) > FOO = "foo" > loop { > DL::PtrData.new(2*FOO.object_id) ; > } It feels too dangerous too me. Index: ext/dl/dl.c =================================================================== RCS file: /cvs/ruby/src/ruby/ext/dl/dl.c,v retrieving revision 1.20 diff -U2 -p -d -r1.20 dl.c --- ext/dl/dl.c 16 Jun 2003 07:25:38 -0000 1.20 +++ ext/dl/dl.c 21 May 2004 02:16:53 -0000 @@ -1,2 +1,3 @@ +/* -*- mode: C; c-file-style: "gnu" -*- /* * $Id: dl.c,v 1.20 2003/06/16 07:25:38 usa Exp $ @@ -165,7 +166,7 @@ dlsizeof(const char *cstr) for (i=0; iptr), RSTRING(str)->len, dlfree); + len = RSTRING(str)->len; + ptr = memcpy(dlmalloc(len + 1), RSTRING(str)->ptr, len + 1); + ((char *)ptr)[len] = '\0'; + return rb_dlptr_new(ptr, len, dlfree); } Index: ext/dl/dl.h =================================================================== RCS file: /cvs/ruby/src/ruby/ext/dl/dl.h,v retrieving revision 1.9 diff -U2 -p -d -r1.9 dl.h --- ext/dl/dl.h 1 Dec 2003 23:02:44 -0000 1.9 +++ ext/dl/dl.h 21 May 2004 02:19:17 -0000 @@ -1,3 +1,3 @@ -/* -*- C -*- +/* -*- mode: C; c-file-style: "gnu" -*- * $Id: dl.h,v 1.9 2003/12/01 23:02:44 ttate Exp $ */ Index: ext/dl/handle.c =================================================================== RCS file: /cvs/ruby/src/ruby/ext/dl/handle.c,v retrieving revision 1.11 diff -U2 -p -d -r1.11 handle.c --- ext/dl/handle.c 13 Nov 2003 11:46:31 -0000 1.11 +++ ext/dl/handle.c 21 May 2004 02:19:26 -0000 @@ -1,3 +1,3 @@ -/* -*- C -*- +/* -*- mode: C; c-file-style: "gnu" -*- * $Id: handle.c,v 1.11 2003/11/13 11:46:31 ttate Exp $ */ Index: ext/dl/ptr.c =================================================================== RCS file: /cvs/ruby/src/ruby/ext/dl/ptr.c,v retrieving revision 1.19 diff -U2 -p -d -r1.19 ptr.c --- ext/dl/ptr.c 22 Oct 2003 14:29:20 -0000 1.19 +++ ext/dl/ptr.c 21 May 2004 02:24:42 -0000 @@ -1,3 +1,3 @@ -/* -*- C -*- +/* -*- mode: C; c-file-style: "gnu" -*- * $Id: ptr.c,v 1.19 2003/10/22 14:29:20 ttate Exp $ */ @@ -5,10 +5,11 @@ #include #include -#include /* for ruby version code */ +#include #include "dl.h" VALUE rb_cDLPtrData; VALUE rb_mDLMemorySpace; -static VALUE DLMemoryTable; +static st_table *DLMemoryTable; +static int finalized; #ifndef T_SYMBOL @@ -16,17 +17,28 @@ static VALUE DLMemoryTable; #endif -#if RUBY_VERSION_CODE < 171 -static VALUE -rb_hash_delete(VALUE hash, VALUE key) +static void +free_memtbl(void *ptr) { - return rb_funcall(hash, rb_intern("delete"), 1, key); + if (!DLMemoryTable) return; + if (DLMemoryTable->num_entries) { + finalized = 1; + } + else { + st_free_table(DLMemoryTable); + DLMemoryTable = 0; + } } -#endif static void rb_dlmem_delete(void *ptr) { - rb_secure(4); - rb_hash_delete(DLMemoryTable, DLLONG2NUM(ptr)); + st_data_t key = (st_data_t)ptr; + + if (!DLMemoryTable) return; + st_delete(DLMemoryTable, &key, 0); + if (finalized && !DLMemoryTable->num_entries) { + st_free_table(DLMemoryTable); + DLMemoryTable = 0; + } } @@ -34,9 +46,7 @@ static void rb_dlmem_aset(void *ptr, VALUE obj) { - if (obj == Qnil) { - rb_dlmem_delete(ptr); - } - else{ - rb_hash_aset(DLMemoryTable, DLLONG2NUM(ptr), DLLONG2NUM(obj)); + st_data_t val; + if (!st_lookup(DLMemoryTable, (st_data_t)ptr, &val) || (VALUE)val != obj) { + st_add_direct(DLMemoryTable, (st_data_t)ptr, (st_data_t)obj); } } @@ -45,8 +55,9 @@ static VALUE rb_dlmem_aref(void *ptr) { - VALUE val; + st_data_t val; - val = rb_hash_aref(DLMemoryTable, DLLONG2NUM(ptr)); - return val == Qnil ? Qnil : (VALUE)DLNUM2LONG(val); + if (!st_lookup(DLMemoryTable, (st_data_t)ptr, &val)) + return Qnil; + return (VALUE)val; } @@ -70,4 +81,5 @@ dlptr_free(struct ptr_data *data) if (data->ssize) dlfree(data->ssize); if (data->ids) dlfree(data->ids); + dlfree(data); } @@ -95,5 +107,5 @@ rb_dlptr_new2(VALUE klass, void *ptr, lo if (ptr) { val = rb_dlmem_aref(ptr); - if (val == Qnil) { + if (NIL_P(val)) { val = Data_Make_Struct(klass, struct ptr_data, 0, dlptr_free, data); @@ -150,5 +162,5 @@ rb_dlptr2cptr(VALUE val) ptr = data->ptr; } - else if (val == Qnil) { + else if (NIL_P(val)) { ptr = NULL; } @@ -190,4 +202,5 @@ rb_dlptr_initialize(int argc, VALUE argv long s = 0; + rb_secure(4); switch (rb_scan_args(argc, argv, "12", &ptr, &size, &sym)) { case 1: @@ -209,11 +222,17 @@ rb_dlptr_initialize(int argc, VALUE argv if (p) { Data_Get_Struct(self, struct ptr_data, data); - if (data->ptr && data->free) { - /* Free previous memory. Use of inappropriate initialize may cause SEGV. */ - (*(data->free))(data->ptr); + if (data->ptr) { + rb_dlmem_delete(data->ptr); + if (data->free) { + /* Free previous memory. Use of inappropriate initialize may cause SEGV. */ + (*data->free)(data->ptr); + } } data->ptr = p; data->size = s; data->free = f; + if (p) { + if (NIL_P(rb_dlmem_aref(p))) rb_dlmem_aset(p, self); + } } @@ -526,5 +545,5 @@ rb_dlptr_define_data_type(int argc, VALU Data_Get_Struct(self, struct ptr_data, data); - if (argc == 1 || (argc == 2 && type == Qnil)) { + if (NIL_P(type)) { if (NUM2INT(data_type) == DLPTR_CTYPE_UNKNOWN) { data->ctype = DLPTR_CTYPE_UNKNOWN; @@ -570,7 +589,7 @@ rb_dlptr_define_data_type(int argc, VALU data->stype[i] = *ctype; ctype ++; - if (isdigit(*ctype)) { + if (ISDIGIT(*ctype)) { char *p, *d; - for (p=ctype; isdigit(*p); p++) ; + for (p=ctype; ISDIGIT(*p); p++) ; d = ALLOCA_N(char, p - ctype + 1); strncpy(d, ctype, p - ctype); @@ -867,5 +886,5 @@ rb_dlptr_aset(int argc, VALUE argv[], VA src = RSTRING(val)->ptr; len = RSTRING(val)->len; - if (num == Qnil) { + if (NIL_P(num)) { memcpy(dst, src, len); } @@ -1008,12 +1027,9 @@ rb_dlptr_size(int argc, VALUE argv[], VA } -static VALUE -dlmem_each_i(VALUE assoc, void *data) +static int +dlmem_each_i(st_data_t key, st_data_t val, st_data_t arg) { - VALUE key, val; - key = rb_ary_entry(assoc, 0); - val = rb_ary_entry(assoc, 1); - rb_yield(rb_assoc_new(key,(VALUE)DLNUM2LONG(val))); - return Qnil; + rb_yield_values(DLLONG2NUM(key), (VALUE)val); + return ST_CONTINUE; } @@ -1021,5 +1037,5 @@ VALUE rb_dlmem_each(VALUE self) { - rb_iterate(rb_each, DLMemoryTable, dlmem_each_i, 0); + st_foreach(DLMemoryTable, dlmem_each_i, 0); return Qnil; } @@ -1060,6 +1076,8 @@ Init_dlptr() rb_mDLMemorySpace = rb_define_module_under(rb_mDL, "MemorySpace"); - DLMemoryTable = rb_hash_new(); - rb_define_const(rb_mDLMemorySpace, "MemoryTable", DLMemoryTable); + rb_extend_object(rb_mDLMemorySpace, rb_mEnumerable); + DLMemoryTable = st_init_numtable(); + rb_define_const(rb_mDLMemorySpace, "MemoryTable", + Data_Wrap_Struct(rb_cData, 0, free_memtbl, DLMemoryTable)); rb_define_module_function(rb_mDLMemorySpace, "each", rb_dlmem_each, 0); } Index: ext/dl/sym.c =================================================================== RCS file: /cvs/ruby/src/ruby/ext/dl/sym.c,v retrieving revision 1.24 diff -U2 -p -d -r1.24 sym.c --- ext/dl/sym.c 11 Dec 2003 02:39:58 -0000 1.24 +++ ext/dl/sym.c 21 May 2004 02:19:13 -0000 @@ -1,3 +1,3 @@ -/* -*- C -*- +/* -*- mode: C; c-file-style: "gnu" -*- * $Id: sym.c,v 1.24 2003/12/11 02:39:58 nobu Exp $ */ Index: ext/dl/lib/dl/import.rb =================================================================== RCS file: /cvs/ruby/src/ruby/ext/dl/lib/dl/import.rb,v retrieving revision 1.10 diff -U2 -p -d -r1.10 import.rb --- ext/dl/lib/dl/import.rb 16 Oct 2003 17:47:17 -0000 1.10 +++ ext/dl/lib/dl/import.rb 21 May 2004 02:21:32 -0000 @@ -10,13 +10,9 @@ module DL module Internal def init_types() - if( !@types ) - @types = ::DL::Types.new - end + @types ||= ::DL::Types.new end def init_sym() - if( !@SYM ) - @SYM = {} - end + @SYM ||= {} end -- Nobu Nakada