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 /ext | |
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
Diffstat (limited to 'ext')
-rw-r--r-- | ext/syck/rubyext.c | 175 | ||||
-rw-r--r-- | ext/syck/syck.h | 2 |
2 files changed, 76 insertions, 101 deletions
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)) |