diff options
author | ocean <ocean@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2005-12-20 04:13:26 +0000 |
---|---|---|
committer | ocean <ocean@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2005-12-20 04:13:26 +0000 |
commit | 534d30887d1fab63dadc0b392cda32d6f319e3b9 (patch) | |
tree | 0659845fb9f727b8322fc13672b4a33db5aa8983 | |
parent | 3b7777959a81b8f16c991518e0daf770f882bf35 (diff) |
* ext/syck/rubyext.c: fixed GC problem (backported HEAD 1.55 - 1.62)
[ruby-dev:27839]
* ext/syck/syck.h (S_FREE): small hack. no need to check if pointer is
NULL or not before S_FREE.
* st.c: uses malloc instead of xmalloc to avoid GC. syck uses st_insert
in gram.c to insert node from rb_syck_bad_anchor_handler into
SyckParser's hash table. if GC occurs in st_insert, it's not under
SyckParser's mark system yet. so RString can be released wrongly.
[ruby-dev:28057]
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_1_8@9722 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r-- | ChangeLog | 14 | ||||
-rw-r--r-- | ext/syck/rubyext.c | 175 | ||||
-rw-r--r-- | ext/syck/syck.h | 2 | ||||
-rw-r--r-- | st.c | 36 |
4 files changed, 102 insertions, 125 deletions
@@ -1,3 +1,17 @@ +Tue Dec 20 13:11:59 2005 Hirokazu Yamamoto <ocean@m2.ccsnet.ne.jp> + + * ext/syck/rubyext.c: fixed GC problem (backported HEAD 1.55 - 1.62) + [ruby-dev:27839] + + * ext/syck/syck.h (S_FREE): small hack. no need to check if pointer is + NULL or not before S_FREE. + + * st.c: uses malloc instead of xmalloc to avoid GC. syck uses st_insert + in gram.c to insert node from rb_syck_bad_anchor_handler into + SyckParser's hash table. if GC occurs in st_insert, it's not under + SyckParser's mark system yet. so RString can be released wrongly. + [ruby-dev:28057] + Tue Dec 20 12:53:23 2005 why the lucky stiff <why@ruby-lang.org> * ext/syck/rubyext.c (syck_emitter_reset): to ensure compatibility diff --git a/ext/syck/rubyext.c b/ext/syck/rubyext.c index 027211539b..272cfd2571 100644 --- a/ext/syck/rubyext.c +++ b/ext/syck/rubyext.c @@ -75,7 +75,7 @@ void rb_syck_err_handler _((SyckParser *, char *)); SyckNode * rb_syck_bad_anchor_handler _((SyckParser *, char *)); void rb_syck_output_handler _((SyckEmitter *, char *, long)); void rb_syck_emitter_handler _((SyckEmitter *, st_data_t)); -int syck_parser_assign_io _((SyckParser *, VALUE)); +int syck_parser_assign_io _((SyckParser *, VALUE *)); VALUE syck_scalar_alloc _((VALUE class)); VALUE syck_seq_alloc _((VALUE class)); VALUE syck_map_alloc _((VALUE class)); @@ -107,7 +107,7 @@ rb_syck_compile(self, port) bytestring_t *sav; SyckParser *parser = syck_new_parser(); - taint = syck_parser_assign_io(parser, port); + taint = syck_parser_assign_io(parser, &port); syck_parser_handler( parser, syck_yaml2byte_handler ); syck_parser_error_handler( parser, NULL ); syck_parser_implicit_typing( parser, 0 ); @@ -122,7 +122,7 @@ rb_syck_compile(self, port) syck_free_parser( parser ); - bc = rb_str_new2( ret ); + bc = rb_str_new2( ret ); S_FREE( ret ); if ( taint ) OBJ_TAINT( bc ); return bc; } @@ -164,12 +164,12 @@ rb_syck_io_str_read( char *buf, SyckIoStr *str, long max_size, long skip ) * (returns tainted? boolean) */ int -syck_parser_assign_io(parser, port) +syck_parser_assign_io(parser, pport) SyckParser *parser; - VALUE port; + VALUE *pport; { int taint = Qtrue; - VALUE tmp; + VALUE tmp, port = *pport; if (!NIL_P(tmp = rb_check_string_type(port))) { taint = OBJ_TAINTED(port); /* original taintedness */ port = tmp; @@ -184,6 +184,7 @@ syck_parser_assign_io(parser, port) else { rb_raise(rb_eTypeError, "instance of IO needed"); } + *pport = port; return taint; } @@ -265,12 +266,11 @@ rb_syck_mktime(str, len) ptr += 2; if ( len > ptr - str && *ptr == '.' ) { - char *padded = syck_strndup( "000000", 6 ); + char padded[] = "000000"; char *end = ptr + 1; while ( isdigit( *end ) ) end++; MEMCPY(padded, ptr + 1, char, end - (ptr + 1)); usec = strtol(padded, NULL, 10); - S_FREE(padded); } else { @@ -595,6 +595,8 @@ yaml_org_handler( n, ref ) return transferred; } +static void syck_node_mark( SyckNode *n ); + /* * {native mode} node handler * - Converts data into native Ruby types @@ -703,6 +705,13 @@ syck_set_model( p, input, model ) syck_parser_bad_anchor_handler( parser, rb_syck_bad_anchor_handler ); } +static int +syck_st_mark_nodes( char *key, SyckNode *n, char *arg ) +{ + if ( n != (void *)1 ) syck_node_mark( n ); + return ST_CONTINUE; +} + /* * mark parser nodes */ @@ -710,14 +719,20 @@ static void syck_mark_parser(parser) SyckParser *parser; { - struct parser_xtra *bonus; - rb_gc_mark(parser->root); - rb_gc_mark(parser->root_on_error); - if ( parser->bonus != NULL ) + struct parser_xtra *bonus = (struct parser_xtra *)parser->bonus; + rb_gc_mark_maybe(parser->root); + rb_gc_mark_maybe(parser->root_on_error); + rb_gc_mark( bonus->data ); + rb_gc_mark( bonus->proc ); + rb_gc_mark( bonus->resolver ); + + if ( parser->anchors != NULL ) { - bonus = (struct parser_xtra *)parser->bonus; - rb_gc_mark( bonus->data ); - rb_gc_mark( bonus->proc ); + st_foreach( parser->anchors, syck_st_mark_nodes, 0 ); + } + if ( parser->bad_anchors != NULL ) + { + st_foreach( parser->bad_anchors, syck_st_mark_nodes, 0 ); } } @@ -728,8 +743,7 @@ void rb_syck_free_parser(p) SyckParser *p; { - struct parser_xtra *bonus = (struct parser_xtra *)p->bonus; - if ( bonus != NULL ) S_FREE( bonus ); + S_FREE( p->bonus ); syck_free_parser(p); } @@ -744,6 +758,9 @@ syck_parser_s_alloc(class) VALUE pobj; SyckParser *parser = syck_new_parser(); + parser->bonus = S_ALLOC( struct parser_xtra ); + S_MEMZERO( parser->bonus, struct parser_xtra, 1 ); + pobj = Data_Wrap_Struct( class, syck_mark_parser, rb_syck_free_parser, parser ); syck_parser_set_root_on_error( parser, Qnil ); @@ -815,8 +832,7 @@ syck_parser_load(argc, argv, self) { VALUE port, proc, model, input; SyckParser *parser; - struct parser_xtra *bonus = S_ALLOC_N( struct parser_xtra, 1 ); - volatile VALUE hash; /* protect from GC */ + struct parser_xtra *bonus; rb_scan_args(argc, argv, "11", &port, &proc); @@ -825,13 +841,12 @@ syck_parser_load(argc, argv, self) Data_Get_Struct(self, SyckParser, parser); syck_set_model( self, input, model ); - bonus->taint = syck_parser_assign_io(parser, port); - bonus->data = hash = rb_hash_new(); + bonus = (struct parser_xtra *)parser->bonus; + bonus->taint = syck_parser_assign_io(parser, &port); + bonus->data = rb_hash_new(); bonus->resolver = rb_attr_get( self, s_resolver ); if ( NIL_P( proc ) ) bonus->proc = 0; else bonus->proc = proc; - - parser->bonus = (void *)bonus; return syck_parse( parser ); } @@ -847,8 +862,7 @@ syck_parser_load_documents(argc, argv, self) { VALUE port, proc, v, input, model; SyckParser *parser; - struct parser_xtra *bonus = S_ALLOC_N( struct parser_xtra, 1 ); - volatile VALUE hash; + struct parser_xtra *bonus; rb_scan_args(argc, argv, "1&", &port, &proc); @@ -857,15 +871,15 @@ syck_parser_load_documents(argc, argv, self) Data_Get_Struct(self, SyckParser, parser); syck_set_model( self, input, model ); - bonus->taint = syck_parser_assign_io(parser, port); + bonus = (struct parser_xtra *)parser->bonus; + bonus->taint = syck_parser_assign_io(parser, &port); bonus->resolver = rb_attr_get( self, s_resolver ); bonus->proc = 0; - parser->bonus = (void *)bonus; while ( 1 ) { /* Reset hash for tracking nodes */ - bonus->data = hash = rb_hash_new(); + bonus->data = rb_hash_new(); /* Parse a document */ v = syck_parse( parser ); @@ -1187,10 +1201,9 @@ syck_resolver_tagurize( self, val ) if ( !NIL_P(tmp) ) { - char *taguri; - val = tmp; - taguri = syck_type_id_to_uri( RSTRING(val)->ptr ); - return rb_str_new2( taguri ); + char *taguri = syck_type_id_to_uri( RSTRING(tmp)->ptr ); + val = rb_str_new2( taguri ); + S_FREE( taguri ); } return val; @@ -1380,6 +1393,7 @@ syck_node_mark( n ) SyckNode *n; { int i; + rb_gc_mark_maybe( n->id ); switch ( n->kind ) { case syck_seq_kind: @@ -1397,42 +1411,9 @@ syck_node_mark( n ) } break; } -} - -/* - * Don't free Ruby data, Ruby will do that - */ -void -rb_syck_free_node( SyckNode *n ) -{ - switch ( n->kind ) - { - case syck_str_kind: - S_FREE( n->data.str ); - n->data.str = NULL; - break; - - case syck_seq_kind: - if ( n->data.list != NULL ) - { - S_FREE( n->data.list->items ); - S_FREE( n->data.list ); - n->data.list = NULL; - } - break; - - case syck_map_kind: - if ( n->data.pairs != NULL ) - { - S_FREE( n->data.pairs->keys ); - S_FREE( n->data.pairs->values ); - S_FREE( n->data.pairs ); - n->data.pairs = NULL; - } - break; - } - - S_FREE( n ); +#if 0 /* maybe needed */ + if ( n->shortcut ) syck_node_mark( n->shortcut ); /* caution: maybe cyclic */ +#endif } /* @@ -1443,7 +1424,7 @@ syck_scalar_alloc( class ) VALUE class; { SyckNode *node = syck_alloc_str(); - VALUE obj = Data_Wrap_Struct( class, syck_node_mark, rb_syck_free_node, node ); + VALUE obj = Data_Wrap_Struct( class, syck_node_mark, syck_free_node, node ); node->id = obj; return obj; } @@ -1512,7 +1493,7 @@ syck_scalar_value_set( self, val ) Data_Get_Struct( self, SyckNode, node ); StringValue( val ); - node->data.str->ptr = RSTRING(val)->ptr; + node->data.str->ptr = syck_strndup( RSTRING(val)->ptr, RSTRING(val)->len ); node->data.str->len = RSTRING(val)->len; node->data.str->style = scalar_none; @@ -1530,7 +1511,7 @@ syck_seq_alloc( class ) SyckNode *node; VALUE obj; node = syck_alloc_seq(); - obj = Data_Wrap_Struct( class, syck_node_mark, rb_syck_free_node, node ); + obj = Data_Wrap_Struct( class, syck_node_mark, syck_free_node, node ); node->id = obj; return obj; } @@ -1629,7 +1610,7 @@ syck_map_alloc( class ) SyckNode *node; VALUE obj; node = syck_alloc_map(); - obj = Data_Wrap_Struct( class, syck_node_mark, rb_syck_free_node, node ); + obj = Data_Wrap_Struct( class, syck_node_mark, syck_free_node, node ); node->id = obj; return obj; } @@ -1759,8 +1740,7 @@ syck_node_init_copy( copy, orig ) if ( copy == orig ) return copy; - if ( TYPE( orig ) != T_DATA || - RDATA( orig )->dfree != ( RUBY_DATA_FUNC )rb_syck_free_node ) + if ( TYPE( orig ) != T_DATA ) { rb_raise( rb_eTypeError, "wrong argument type" ); } @@ -1781,12 +1761,11 @@ syck_node_type_id_set( self, type_id ) SyckNode *node; Data_Get_Struct( self, SyckNode, node ); - if ( node->type_id != NULL ) S_FREE( node->type_id ); + S_FREE( node->type_id ); - if ( NIL_P( type_id ) ) { - node->type_id = NULL; - } else { - node->type_id = StringValuePtr( type_id ); + if ( !NIL_P( type_id ) ) { + StringValue( type_id ); + node->type_id = syck_strndup( RSTRING(type_id)->ptr, RSTRING(type_id)->len ); } rb_iv_set( self, "@type_id", type_id ); @@ -1804,13 +1783,14 @@ syck_node_transform( self ) SyckNode *n; SyckNode *orig_n; Data_Get_Struct(self, SyckNode, orig_n); + t = Data_Wrap_Struct( cNode, syck_node_mark, syck_free_node, 0 ); switch (orig_n->kind) { case syck_map_kind: { int i; - n = syck_alloc_map(); + DATA_PTR(t) = n = syck_alloc_map(); for ( i = 0; i < orig_n->data.pairs->idx; i++ ) { syck_map_add( n, rb_funcall( syck_map_read( orig_n, map_key, i ), s_transform, 0 ), @@ -1822,7 +1802,7 @@ syck_node_transform( self ) case syck_seq_kind: { int i; - n = syck_alloc_seq(); + DATA_PTR(t) = n = syck_alloc_seq(); for ( i = 0; i < orig_n->data.list->idx; i++ ) { syck_seq_add( n, rb_funcall( syck_seq_read( orig_n, i ), s_transform, 0 ) ); @@ -1831,7 +1811,7 @@ syck_node_transform( self ) break; case syck_str_kind: - n = syck_new_str2( orig_n->data.str->ptr, orig_n->data.str->len, orig_n->data.str->style ); + DATA_PTR(t) = n = syck_new_str2( orig_n->data.str->ptr, orig_n->data.str->len, orig_n->data.str->style ); break; } @@ -1843,11 +1823,8 @@ syck_node_transform( self ) { n->anchor = syck_strndup( orig_n->anchor, strlen( orig_n->anchor ) ); } - t = Data_Wrap_Struct( cNode, NULL, NULL, n ); n->id = t; - t = rb_funcall( oDefaultResolver, s_node_import, 1, t ); - syck_free_node( n ); - return t; + return rb_funcall( oDefaultResolver, s_node_import, 1, t ); } /* @@ -1942,13 +1919,10 @@ static void syck_mark_emitter(emitter) SyckEmitter *emitter; { - struct emitter_xtra *bonus; - if ( emitter->bonus != NULL ) - { - bonus = (struct emitter_xtra *)emitter->bonus; - rb_gc_mark( bonus->data ); - rb_gc_mark( bonus->port ); - } + struct emitter_xtra *bonus = (struct emitter_xtra *)emitter->bonus; + rb_gc_mark( bonus->oid ); + rb_gc_mark( bonus->data ); + rb_gc_mark( bonus->port ); } /* @@ -1958,8 +1932,7 @@ void rb_syck_free_emitter(e) SyckEmitter *e; { - struct emitter_xtra *bonus = (struct emitter_xtra *)e->bonus; - if ( bonus != NULL ) S_FREE( bonus ); + S_FREE( e->bonus ); syck_free_emitter(e); } @@ -1974,6 +1947,9 @@ syck_emitter_s_alloc(class) VALUE pobj; SyckEmitter *emitter = syck_new_emitter(); + emitter->bonus = S_ALLOC( struct emitter_xtra ); + S_MEMZERO( emitter->bonus, struct emitter_xtra, 1 ); + pobj = Data_Wrap_Struct( class, syck_mark_emitter, rb_syck_free_emitter, emitter ); syck_emitter_handler( emitter, rb_syck_emitter_handler ); syck_output_handler( emitter, rb_syck_output_handler ); @@ -1994,15 +1970,13 @@ syck_emitter_reset( argc, argv, self ) VALUE options, tmp; SyckEmitter *emitter; struct emitter_xtra *bonus; - volatile VALUE hash; /* protect from GC */ Data_Get_Struct(self, SyckEmitter, emitter); bonus = (struct emitter_xtra *)emitter->bonus; - if ( bonus != NULL ) S_FREE( bonus ); - bonus = S_ALLOC_N( struct emitter_xtra, 1 ); + bonus->oid = Qnil; bonus->port = rb_str_new2( "" ); - bonus->data = hash = rb_hash_new(); + bonus->data = rb_hash_new(); if (rb_scan_args(argc, argv, "01", &options) == 0) { @@ -2024,7 +1998,6 @@ syck_emitter_reset( argc, argv, self ) } emitter->headless = 0; - emitter->bonus = (void *)bonus; rb_ivar_set(self, s_level, INT2FIX(0)); rb_ivar_set(self, s_resolver, Qnil); return self; @@ -2252,10 +2225,12 @@ Init_syck() rb_define_method( cResolver, "node_import", syck_resolver_node_import, 1 ); rb_define_method( cResolver, "tagurize", syck_resolver_tagurize, 1 ); + rb_global_variable( &oDefaultResolver ); oDefaultResolver = rb_funcall( cResolver, rb_intern( "new" ), 0 ); rb_define_singleton_method( oDefaultResolver, "node_import", syck_defaultresolver_node_import, 1 ); rb_define_singleton_method( oDefaultResolver, "detect_implicit", syck_defaultresolver_detect_implicit, 1 ); rb_define_const( rb_syck, "DefaultResolver", oDefaultResolver ); + rb_global_variable( &oGenericResolver ); oGenericResolver = rb_funcall( cResolver, rb_intern( "new" ), 0 ); rb_define_singleton_method( oGenericResolver, "node_import", syck_genericresolver_node_import, 1 ); rb_define_const( rb_syck, "GenericResolver", oGenericResolver ); diff --git a/ext/syck/syck.h b/ext/syck/syck.h index f7ac39fc94..c49f740173 100644 --- a/ext/syck/syck.h +++ b/ext/syck/syck.h @@ -51,7 +51,7 @@ extern "C" { #define S_ALLOC_N(type,n) (type*)malloc(sizeof(type)*(n)) #define S_ALLOC(type) (type*)malloc(sizeof(type)) #define S_REALLOC_N(var,type,n) (var)=(type*)realloc((char*)(var),sizeof(type)*(n)) -#define S_FREE(n) free(n); n = NULL; +#define S_FREE(n) if (n) { free(n); n = NULL; } #define S_ALLOCA_N(type,n) (type*)alloca(sizeof(type)*(n)) @@ -46,22 +46,10 @@ static struct st_hash_type type_strhash = { strhash, }; -#ifdef RUBY_PLATFORM -#define xmalloc ruby_xmalloc -#define xcalloc ruby_xcalloc -#define xrealloc ruby_xrealloc -#define xfree ruby_xfree - -void *xmalloc(long); -void *xcalloc(long, long); -void *xrealloc(void *, long); -void xfree(void *); -#endif - static void rehash(st_table *); -#define alloc(type) (type*)xmalloc((unsigned)sizeof(type)) -#define Calloc(n,s) (char*)xcalloc((n),(s)) +#define alloc(type) (type*)malloc((unsigned)sizeof(type)) +#define Calloc(n,s) (char*)calloc((n),(s)) #define EQUAL(table,x,y) ((x)==(y) || (*table->type->compare)((x),(y)) == 0) @@ -216,12 +204,12 @@ st_free_table(table) ptr = table->bins[i]; while (ptr != 0) { next = ptr->next; - xfree(ptr); + free(ptr); ptr = next; } } - xfree(table->bins); - xfree(table); + free(table->bins); + free(table); } #define PTR_NOT_EQUAL(table, ptr, hash_val, key) \ @@ -340,7 +328,7 @@ rehash(table) ptr = next; } } - xfree(table->bins); + free(table->bins); table->num_bins = new_num_bins; table->bins = new_bins; } @@ -363,7 +351,7 @@ st_copy(old_table) Calloc((unsigned)num_bins, sizeof(st_table_entry*)); if (new_table->bins == 0) { - xfree(new_table); + free(new_table); return 0; } @@ -373,8 +361,8 @@ st_copy(old_table) while (ptr != 0) { entry = alloc(st_table_entry); if (entry == 0) { - xfree(new_table->bins); - xfree(new_table); + free(new_table->bins); + free(new_table); return 0; } *entry = *ptr; @@ -409,7 +397,7 @@ st_delete(table, key, value) table->num_entries--; if (value != 0) *value = ptr->record; *key = ptr->key; - xfree(ptr); + free(ptr); return 1; } @@ -420,7 +408,7 @@ st_delete(table, key, value) table->num_entries--; if (value != 0) *value = tmp->record; *key = tmp->key; - xfree(tmp); + free(tmp); return 1; } } @@ -520,7 +508,7 @@ st_foreach(table, func, arg) last->next = ptr->next; } ptr = ptr->next; - xfree(tmp); + free(tmp); table->num_entries--; } } |