From a5fbe31e3b8fae020a78f0de4f82e5fe30ebcd8e Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Wed, 5 Aug 2020 00:39:38 +0200 Subject: [PATCH 1/9] Convert PG::TypeMap derivations to TypedData objects Classic T_DATA objects are deprecated since ruby-2.0. On the other hand TypedData objects can be checked more easy and allows some new features. --- ext/pg.h | 1 + ext/pg_connection.c | 29 ++++++++++----------- ext/pg_copy_coder.c | 4 +-- ext/pg_record_coder.c | 4 +-- ext/pg_result.c | 15 +++++------ ext/pg_tuple.c | 2 +- ext/pg_type_map.c | 29 +++++++++++++++------ ext/pg_type_map_all_strings.c | 16 +++++++++++- ext/pg_type_map_by_class.c | 30 ++++++++++++++++------ ext/pg_type_map_by_column.c | 41 ++++++++++++++++++++---------- ext/pg_type_map_by_mri_type.c | 28 +++++++++++++++----- ext/pg_type_map_by_oid.c | 40 +++++++++++++++++++---------- ext/pg_type_map_in_ruby.c | 48 ++++++++++++++++++++++++----------- 13 files changed, 191 insertions(+), 96 deletions(-) diff --git a/ext/pg.h b/ext/pg.h index adb3214e3..61e9b0f28 100644 --- a/ext/pg.h +++ b/ext/pg.h @@ -220,6 +220,7 @@ typedef struct { } convs[0]; } t_tmbc; +extern const rb_data_type_t pg_typemap_type; #include "gvl_wrappers.h" diff --git a/ext/pg_connection.c b/ext/pg_connection.c index 6d3a1a3bb..e47c11609 100644 --- a/ext/pg_connection.c +++ b/ext/pg_connection.c @@ -1095,7 +1095,7 @@ alloc_query_params(struct query_params_data *paramsData) Check_Type(paramsData->params, T_ARRAY); - p_typemap = DATA_PTR( paramsData->typemap ); + p_typemap = RTYPEDDATA_DATA( paramsData->typemap ); p_typemap->funcs.fit_to_query( paramsData->typemap, paramsData->params ); paramsData->heap_pool = Qnil; @@ -1227,12 +1227,11 @@ pgconn_query_assign_typemap( VALUE self, struct query_params_data *paramsData ) /* Use default typemap for queries. It's type is checked when assigned. */ paramsData->typemap = pg_get_connection(self)->type_map_for_queries; }else{ + t_typemap *tm; + UNUSED(tm); + /* Check type of method param */ - if ( !rb_obj_is_kind_of(paramsData->typemap, rb_cTypeMap) ) { - rb_raise( rb_eTypeError, "wrong argument type %s (expected kind of PG::TypeMap)", - rb_obj_classname( paramsData->typemap ) ); - } - Check_Type( paramsData->typemap, T_DATA ); + TypedData_Get_Struct(paramsData->typemap, t_typemap, &pg_typemap_type, tm); } } @@ -3916,12 +3915,12 @@ static VALUE pgconn_type_map_for_queries_set(VALUE self, VALUE typemap) { t_pg_connection *this = pg_get_connection( self ); + t_typemap *tm; + UNUSED(tm); + + /* Check type of method param */ + TypedData_Get_Struct(typemap, t_typemap, &pg_typemap_type, tm); - if ( !rb_obj_is_kind_of(typemap, rb_cTypeMap) ) { - rb_raise( rb_eTypeError, "wrong argument type %s (expected kind of PG::TypeMap)", - rb_obj_classname( typemap ) ); - } - Check_Type(typemap, T_DATA); this->type_map_for_queries = typemap; return typemap; @@ -3956,12 +3955,10 @@ static VALUE pgconn_type_map_for_results_set(VALUE self, VALUE typemap) { t_pg_connection *this = pg_get_connection( self ); + t_typemap *tm; + UNUSED(tm); - if ( !rb_obj_is_kind_of(typemap, rb_cTypeMap) ) { - rb_raise( rb_eTypeError, "wrong argument type %s (expected kind of PG::TypeMap)", - rb_obj_classname( typemap ) ); - } - Check_Type(typemap, T_DATA); + TypedData_Get_Struct(typemap, t_typemap, &pg_typemap_type, tm); this->type_map_for_results = typemap; return typemap; diff --git a/ext/pg_copy_coder.c b/ext/pg_copy_coder.c index 195997232..19222e0ca 100644 --- a/ext/pg_copy_coder.c +++ b/ext/pg_copy_coder.c @@ -188,7 +188,7 @@ pg_text_enc_copy_row(t_pg_coder *conv, VALUE value, char *out, VALUE *intermedia char *current_out; char *end_capa_ptr; - p_typemap = DATA_PTR( this->typemap ); + p_typemap = RTYPEDDATA_DATA( this->typemap ); p_typemap->funcs.fit_to_query( this->typemap, value ); /* Allocate a new string with embedded capacity and realloc exponential when needed. */ @@ -376,7 +376,7 @@ pg_text_dec_copy_row(t_pg_coder *conv, const char *input_line, int len, int _tup char *end_capa_ptr; t_typemap *p_typemap; - p_typemap = DATA_PTR( this->typemap ); + p_typemap = RTYPEDDATA_DATA( this->typemap ); expected_fields = p_typemap->funcs.fit_to_copy_get( this->typemap ); /* The received input string will probably have this->nfields fields. */ diff --git a/ext/pg_record_coder.c b/ext/pg_record_coder.c index 17cc84e16..a718ee11b 100644 --- a/ext/pg_record_coder.c +++ b/ext/pg_record_coder.c @@ -156,7 +156,7 @@ pg_text_enc_record(t_pg_coder *conv, VALUE value, char *out, VALUE *intermediate char *current_out; char *end_capa_ptr; - p_typemap = DATA_PTR( this->typemap ); + p_typemap = RTYPEDDATA_DATA( this->typemap ); p_typemap->funcs.fit_to_query( this->typemap, value ); /* Allocate a new string with embedded capacity and realloc exponential when needed. */ @@ -358,7 +358,7 @@ pg_text_dec_record(t_pg_coder *conv, char *input_line, int len, int _tuple, int char *end_capa_ptr; t_typemap *p_typemap; - p_typemap = DATA_PTR( this->typemap ); + p_typemap = RTYPEDDATA_DATA( this->typemap ); expected_fields = p_typemap->funcs.fit_to_copy_get( this->typemap ); /* The received input string will probably have this->nfields fields. */ diff --git a/ext/pg_result.c b/ext/pg_result.c index 683a550c5..110ce95c6 100644 --- a/ext/pg_result.c +++ b/ext/pg_result.c @@ -191,7 +191,7 @@ pg_new_result2(PGresult *result, VALUE rb_pgconn) this->pgresult = result; this->connection = rb_pgconn; this->typemap = pg_typemap_all_strings; - this->p_typemap = DATA_PTR( this->typemap ); + this->p_typemap = RTYPEDDATA_DATA( this->typemap ); this->nfields = -1; this->tuple_hash = Qnil; this->field_map = Qnil; @@ -202,11 +202,11 @@ pg_new_result2(PGresult *result, VALUE rb_pgconn) t_pg_connection *p_conn = pg_get_connection(rb_pgconn); VALUE typemap = p_conn->type_map_for_results; /* Type check is done when assigned to PG::Connection. */ - t_typemap *p_typemap = DATA_PTR(typemap); + t_typemap *p_typemap = RTYPEDDATA_DATA(typemap); this->enc_idx = p_conn->enc_idx; this->typemap = p_typemap->funcs.fit_to_result( typemap, self ); - this->p_typemap = DATA_PTR( this->typemap ); + this->p_typemap = RTYPEDDATA_DATA( this->typemap ); this->flags = p_conn->flags; } else { this->enc_idx = rb_locale_encindex(); @@ -1325,14 +1325,11 @@ pgresult_type_map_set(VALUE self, VALUE typemap) t_pg_result *this = pgresult_get_this(self); t_typemap *p_typemap; - if ( !rb_obj_is_kind_of(typemap, rb_cTypeMap) ) { - rb_raise( rb_eTypeError, "wrong argument type %s (expected kind of PG::TypeMap)", - rb_obj_classname( typemap ) ); - } - Data_Get_Struct(typemap, t_typemap, p_typemap); + /* Check type of method param */ + TypedData_Get_Struct(typemap, t_typemap, &pg_typemap_type, p_typemap); this->typemap = p_typemap->funcs.fit_to_result( typemap, self ); - this->p_typemap = DATA_PTR( this->typemap ); + this->p_typemap = RTYPEDDATA_DATA( this->typemap ); return typemap; } diff --git a/ext/pg_tuple.c b/ext/pg_tuple.c index 24788c31a..df4ba4ab9 100644 --- a/ext/pg_tuple.c +++ b/ext/pg_tuple.c @@ -172,7 +172,7 @@ pg_tuple_materialize_field(t_pg_tuple *this, int col) VALUE value = this->values[col]; if( value == Qundef ){ - t_typemap *p_typemap = DATA_PTR( this->typemap ); + t_typemap *p_typemap = RTYPEDDATA_DATA( this->typemap ); pgresult_get(this->result); /* make sure we have a valid PGresult object */ value = p_typemap->funcs.typecast_result_value(p_typemap, this->result, this->row_num, col); diff --git a/ext/pg_type_map.c b/ext/pg_type_map.c index 25e14091c..f9ee05459 100644 --- a/ext/pg_type_map.c +++ b/ext/pg_type_map.c @@ -6,6 +6,20 @@ #include "pg.h" +const rb_data_type_t pg_typemap_type = { + "PG::TypeMap", + { + (void (*)(void*))NULL, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + 0, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + VALUE rb_cTypeMap; VALUE rb_mDefaultTypeMappable; static ID s_id_fit_to_query; @@ -75,7 +89,7 @@ pg_typemap_s_allocate( VALUE klass ) VALUE self; t_typemap *this; - self = Data_Make_Struct( klass, t_typemap, NULL, -1, this ); + self = TypedData_Make_Struct( klass, t_typemap, &pg_typemap_type, this ); this->funcs = pg_typemap_funcs; return self; @@ -94,13 +108,12 @@ pg_typemap_s_allocate( VALUE klass ) static VALUE pg_typemap_default_type_map_set(VALUE self, VALUE typemap) { - t_typemap *this = DATA_PTR( self ); + t_typemap *this = RTYPEDDATA_DATA( self ); + t_typemap *tm; + UNUSED(tm); - if ( !rb_obj_is_kind_of(typemap, rb_cTypeMap) ) { - rb_raise( rb_eTypeError, "wrong argument type %s (expected kind of PG::TypeMap)", - rb_obj_classname( typemap ) ); - } - Check_Type(typemap, T_DATA); + /* Check type of method param */ + TypedData_Get_Struct(typemap, t_typemap, &pg_typemap_type, tm); this->default_typemap = typemap; return typemap; @@ -119,7 +132,7 @@ pg_typemap_default_type_map_set(VALUE self, VALUE typemap) static VALUE pg_typemap_default_type_map_get(VALUE self) { - t_typemap *this = DATA_PTR( self ); + t_typemap *this = RTYPEDDATA_DATA( self ); return this->default_typemap; } diff --git a/ext/pg_type_map_all_strings.c b/ext/pg_type_map_all_strings.c index 0e0c64f20..9a721708f 100644 --- a/ext/pg_type_map_all_strings.c +++ b/ext/pg_type_map_all_strings.c @@ -8,6 +8,20 @@ #include "pg.h" +static const rb_data_type_t pg_tmas_type = { + "PG::TypeMapAllStrings", + { + (void (*)(void*))NULL, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + &pg_typemap_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + VALUE rb_cTypeMapAllStrings; VALUE pg_typemap_all_strings; @@ -77,7 +91,7 @@ pg_tmas_s_allocate( VALUE klass ) t_typemap *this; VALUE self; - self = Data_Make_Struct( klass, t_typemap, NULL, -1, this ); + self = TypedData_Make_Struct( klass, t_typemap, &pg_tmas_type, this ); this->funcs.fit_to_result = pg_tmas_fit_to_result; this->funcs.fit_to_query = pg_tmas_fit_to_query; diff --git a/ext/pg_type_map_by_class.c b/ext/pg_type_map_by_class.c index b6b290180..eb0b666ca 100644 --- a/ext/pg_type_map_by_class.c +++ b/ext/pg_type_map_by_class.c @@ -103,7 +103,7 @@ pg_tmbk_typecast_query_param( t_typemap *p_typemap, VALUE param_value, int field p_coder = pg_tmbk_lookup_klass( this, rb_obj_class(param_value), param_value ); if( !p_coder ){ - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); return default_tm->funcs.typecast_query_param( default_tm, param_value, field ); } @@ -113,9 +113,9 @@ pg_tmbk_typecast_query_param( t_typemap *p_typemap, VALUE param_value, int field static VALUE pg_tmbk_fit_to_query( VALUE self, VALUE params ) { - t_tmbk *this = (t_tmbk *)DATA_PTR(self); + t_tmbk *this = (t_tmbk *)RTYPEDDATA_DATA(self); /* Nothing to check at this typemap, but ensure that the default type map fits. */ - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); default_tm->funcs.fit_to_query( this->typemap.default_typemap, params ); return self; } @@ -132,13 +132,27 @@ pg_tmbk_mark( t_tmbk *this ) memset(&this->cache_row, 0, sizeof(this->cache_row)); } +static const rb_data_type_t pg_tmbk_type = { + "PG::TypeMapByClass", + { + (void (*)(void*))pg_tmbk_mark, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + &pg_typemap_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + static VALUE pg_tmbk_s_allocate( VALUE klass ) { t_tmbk *this; VALUE self; - self = Data_Make_Struct( klass, t_tmbk, pg_tmbk_mark, -1, this ); + self = TypedData_Make_Struct( klass, t_tmbk, &pg_tmbk_type, this ); this->typemap.funcs.fit_to_result = pg_typemap_fit_to_result; this->typemap.funcs.fit_to_query = pg_tmbk_fit_to_query; this->typemap.funcs.fit_to_copy_get = pg_typemap_fit_to_copy_get; @@ -152,7 +166,7 @@ pg_tmbk_s_allocate( VALUE klass ) this->self = self; this->klass_to_coder = rb_hash_new(); - /* The cache is properly initialized by Data_Make_Struct(). */ + /* The cache is properly initialized by TypedData_Make_Struct(). */ return self; } @@ -175,7 +189,7 @@ pg_tmbk_s_allocate( VALUE klass ) static VALUE pg_tmbk_aset( VALUE self, VALUE klass, VALUE coder ) { - t_tmbk *this = DATA_PTR( self ); + t_tmbk *this = RTYPEDDATA_DATA( self ); if(NIL_P(coder)){ rb_hash_delete( this->klass_to_coder, klass ); @@ -199,7 +213,7 @@ pg_tmbk_aset( VALUE self, VALUE klass, VALUE coder ) static VALUE pg_tmbk_aref( VALUE self, VALUE klass ) { - t_tmbk *this = DATA_PTR( self ); + t_tmbk *this = RTYPEDDATA_DATA( self ); return rb_hash_lookup(this->klass_to_coder, klass); } @@ -213,7 +227,7 @@ pg_tmbk_aref( VALUE self, VALUE klass ) static VALUE pg_tmbk_coders( VALUE self ) { - t_tmbk *this = DATA_PTR( self ); + t_tmbk *this = RTYPEDDATA_DATA( self ); return rb_obj_freeze(rb_hash_dup(this->klass_to_coder)); } diff --git a/ext/pg_type_map_by_column.c b/ext/pg_type_map_by_column.c index 8abaaeb02..b7262deb3 100644 --- a/ext/pg_type_map_by_column.c +++ b/ext/pg_type_map_by_column.c @@ -16,7 +16,7 @@ static VALUE pg_tmbc_fit_to_result( VALUE self, VALUE result ) { int nfields; - t_tmbc *this = DATA_PTR( self ); + t_tmbc *this = RTYPEDDATA_DATA( self ); t_typemap *default_tm; VALUE sub_typemap; @@ -27,7 +27,7 @@ pg_tmbc_fit_to_result( VALUE self, VALUE result ) } /* Ensure that the default type map fits equaly. */ - default_tm = DATA_PTR( this->typemap.default_typemap ); + default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); sub_typemap = default_tm->funcs.fit_to_result( this->typemap.default_typemap, result ); /* Did the default type return the same object ? */ @@ -42,7 +42,7 @@ pg_tmbc_fit_to_result( VALUE self, VALUE result ) memcpy( p_new_typemap, this, struct_size ); p_new_typemap->typemap.default_typemap = sub_typemap; - DATA_PTR(new_typemap) = p_new_typemap; + RTYPEDDATA_DATA(new_typemap) = p_new_typemap; return new_typemap; } } @@ -51,7 +51,7 @@ static VALUE pg_tmbc_fit_to_query( VALUE self, VALUE params ) { int nfields; - t_tmbc *this = DATA_PTR( self ); + t_tmbc *this = RTYPEDDATA_DATA( self ); t_typemap *default_tm; nfields = (int)RARRAY_LEN( params ); @@ -61,7 +61,7 @@ pg_tmbc_fit_to_query( VALUE self, VALUE params ) } /* Ensure that the default type map fits equaly. */ - default_tm = DATA_PTR( this->typemap.default_typemap ); + default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); default_tm->funcs.fit_to_query( this->typemap.default_typemap, params ); return self; @@ -70,10 +70,10 @@ pg_tmbc_fit_to_query( VALUE self, VALUE params ) static int pg_tmbc_fit_to_copy_get( VALUE self ) { - t_tmbc *this = DATA_PTR( self ); + t_tmbc *this = RTYPEDDATA_DATA( self ); /* Ensure that the default type map fits equaly. */ - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); default_tm->funcs.fit_to_copy_get( this->typemap.default_typemap ); return this->nfields; @@ -107,7 +107,7 @@ pg_tmbc_result_value( t_typemap *p_typemap, VALUE result, int tuple, int field ) } } - default_tm = DATA_PTR( this->typemap.default_typemap ); + default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); return default_tm->funcs.typecast_result_value( default_tm, result, tuple, field ); } @@ -120,7 +120,7 @@ pg_tmbc_typecast_query_param( t_typemap *p_typemap, VALUE param_value, int field t_pg_coder *p_coder = this->convs[field].cconv; if( !p_coder ){ - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); return default_tm->funcs.typecast_query_param( default_tm, param_value, field ); } @@ -142,7 +142,7 @@ pg_tmbc_typecast_copy_get( t_typemap *p_typemap, VALUE field_str, int fieldno, i p_coder = this->convs[fieldno].cconv; if( !p_coder ){ - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); return default_tm->funcs.typecast_copy_get( default_tm, field_str, fieldno, format, enc_idx ); } @@ -194,11 +194,25 @@ pg_tmbc_free( t_tmbc *this ) xfree( this ); } +static const rb_data_type_t pg_tmbc_type = { + "PG::TypeMapByColumn", + { + (void (*)(void*))pg_tmbc_mark, + (void (*)(void*))pg_tmbc_free, + (size_t (*)(const void *))NULL, + }, + &pg_typemap_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + static VALUE pg_tmbc_s_allocate( VALUE klass ) { /* Use pg_typemap_funcs as interim struct until #initialize is called. */ - return Data_Wrap_Struct( klass, pg_tmbc_mark, pg_tmbc_free, (t_tmbc *)&pg_typemap_funcs ); + return TypedData_Wrap_Struct( klass, &pg_tmbc_type, (t_tmbc *)&pg_typemap_funcs ); } VALUE @@ -225,7 +239,6 @@ pg_tmbc_init(VALUE self, VALUE conv_ary) t_tmbc *this; int conv_ary_len; - Check_Type(self, T_DATA); Check_Type(conv_ary, T_ARRAY); conv_ary_len = RARRAY_LEN(conv_ary); this = xmalloc(sizeof(t_tmbc) + sizeof(struct pg_tmbc_converter) * conv_ary_len); @@ -233,7 +246,7 @@ pg_tmbc_init(VALUE self, VALUE conv_ary) this->nfields = 0; this->typemap.funcs = pg_tmbc_funcs; this->typemap.default_typemap = pg_typemap_all_strings; - DATA_PTR(self) = this; + RTYPEDDATA_DATA(self) = this; for(i=0; infields; i++){ diff --git a/ext/pg_type_map_by_mri_type.c b/ext/pg_type_map_by_mri_type.c index 69cde00e9..258e74c6b 100644 --- a/ext/pg_type_map_by_mri_type.c +++ b/ext/pg_type_map_by_mri_type.c @@ -80,7 +80,7 @@ pg_tmbmt_typecast_query_param( t_typemap *p_typemap, VALUE param_value, int fiel } if( !p_coder ){ - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); return default_tm->funcs.typecast_query_param( default_tm, param_value, field ); } @@ -90,9 +90,9 @@ pg_tmbmt_typecast_query_param( t_typemap *p_typemap, VALUE param_value, int fiel static VALUE pg_tmbmt_fit_to_query( VALUE self, VALUE params ) { - t_tmbmt *this = (t_tmbmt *)DATA_PTR(self); + t_tmbmt *this = (t_tmbmt *)RTYPEDDATA_DATA(self); /* Nothing to check at this typemap, but ensure that the default type map fits. */ - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); default_tm->funcs.fit_to_query( this->typemap.default_typemap, params ); return self; } @@ -108,6 +108,20 @@ pg_tmbmt_mark( t_tmbmt *this ) FOR_EACH_MRI_TYPE( GC_MARK_AS_USED ); } +static const rb_data_type_t pg_tmbmt_type = { + "PG::TypeMapByMriType", + { + (void (*)(void*))pg_tmbmt_mark, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + &pg_typemap_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + #define INIT_VARIABLES(type) \ this->coders.coder_##type = NULL; \ this->coders.ask_##type = Qnil; \ @@ -119,7 +133,7 @@ pg_tmbmt_s_allocate( VALUE klass ) t_tmbmt *this; VALUE self; - self = Data_Make_Struct( klass, t_tmbmt, pg_tmbmt_mark, -1, this ); + self = TypedData_Make_Struct( klass, t_tmbmt, &pg_tmbmt_type, this ); this->typemap.funcs.fit_to_result = pg_typemap_fit_to_result; this->typemap.funcs.fit_to_query = pg_tmbmt_fit_to_query; this->typemap.funcs.fit_to_copy_get = pg_typemap_fit_to_copy_get; @@ -188,7 +202,7 @@ pg_tmbmt_s_allocate( VALUE klass ) static VALUE pg_tmbmt_aset( VALUE self, VALUE mri_type, VALUE coder ) { - t_tmbmt *this = DATA_PTR( self ); + t_tmbmt *this = RTYPEDDATA_DATA( self ); char *p_mri_type; p_mri_type = StringValueCStr(mri_type); @@ -220,7 +234,7 @@ static VALUE pg_tmbmt_aref( VALUE self, VALUE mri_type ) { VALUE coder; - t_tmbmt *this = DATA_PTR( self ); + t_tmbmt *this = RTYPEDDATA_DATA( self ); char *p_mri_type; p_mri_type = StringValueCStr(mri_type); @@ -248,7 +262,7 @@ pg_tmbmt_aref( VALUE self, VALUE mri_type ) static VALUE pg_tmbmt_coders( VALUE self ) { - t_tmbmt *this = DATA_PTR( self ); + t_tmbmt *this = RTYPEDDATA_DATA( self ); VALUE hash_coders = rb_hash_new(); FOR_EACH_MRI_TYPE( ADD_TO_HASH ); diff --git a/ext/pg_type_map_by_oid.c b/ext/pg_type_map_by_oid.c index e06d70506..06fc2b225 100644 --- a/ext/pg_type_map_by_oid.c +++ b/ext/pg_type_map_by_oid.c @@ -70,7 +70,7 @@ pg_tmbo_build_type_map_for_result2( t_tmbo *this, PGresult *pgresult ) p_colmap->typemap.default_typemap = pg_typemap_all_strings; colmap = pg_tmbc_allocate(); - DATA_PTR(colmap) = p_colmap; + RTYPEDDATA_DATA(colmap) = p_colmap; for(i=0; ienc_idx ); } - default_tm = DATA_PTR( this->typemap.default_typemap ); + default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); return default_tm->funcs.typecast_result_value( default_tm, result, tuple, field ); } static VALUE pg_tmbo_fit_to_result( VALUE self, VALUE result ) { - t_tmbo *this = DATA_PTR( self ); + t_tmbo *this = RTYPEDDATA_DATA( self ); PGresult *pgresult = pgresult_get( result ); /* Ensure that the default type map fits equaly. */ - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); VALUE sub_typemap = default_tm->funcs.fit_to_result( this->typemap.default_typemap, result ); if( PQntuples( pgresult ) <= this->max_rows_for_online_lookup ){ @@ -137,7 +137,7 @@ pg_tmbo_fit_to_result( VALUE self, VALUE result ) /* The default type map built a new object, so we need to propagate it * and build a copy of this type map. */ VALUE new_typemap = pg_tmbo_s_allocate( rb_cTypeMapByOid ); - t_tmbo *p_new_typemap = DATA_PTR(new_typemap); + t_tmbo *p_new_typemap = RTYPEDDATA_DATA(new_typemap); *p_new_typemap = *this; p_new_typemap->typemap.default_typemap = sub_typemap; return new_typemap; @@ -147,7 +147,7 @@ pg_tmbo_fit_to_result( VALUE self, VALUE result ) * uses a fast array lookup. */ VALUE new_typemap = pg_tmbo_build_type_map_for_result2( this, pgresult ); - t_tmbo *p_new_typemap = DATA_PTR(new_typemap); + t_tmbo *p_new_typemap = RTYPEDDATA_DATA(new_typemap); p_new_typemap->typemap.default_typemap = sub_typemap; return new_typemap; } @@ -164,6 +164,20 @@ pg_tmbo_mark( t_tmbo *this ) } } +static const rb_data_type_t pg_tmbo_type = { + "PG::TypeMapByOid", + { + (void (*)(void*))pg_tmbo_mark, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + &pg_typemap_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + static VALUE pg_tmbo_s_allocate( VALUE klass ) { @@ -171,7 +185,7 @@ pg_tmbo_s_allocate( VALUE klass ) VALUE self; int i; - self = Data_Make_Struct( klass, t_tmbo, pg_tmbo_mark, -1, this ); + self = TypedData_Make_Struct( klass, t_tmbo, &pg_tmbo_type, this ); this->typemap.funcs.fit_to_result = pg_tmbo_fit_to_result; this->typemap.funcs.fit_to_query = pg_typemap_fit_to_query; @@ -205,7 +219,7 @@ static VALUE pg_tmbo_add_coder( VALUE self, VALUE coder ) { VALUE hash; - t_tmbo *this = DATA_PTR( self ); + t_tmbo *this = RTYPEDDATA_DATA( self ); t_pg_coder *p_coder; struct pg_tmbo_oid_cache_entry *p_ce; @@ -243,7 +257,7 @@ pg_tmbo_rm_coder( VALUE self, VALUE format, VALUE oid ) { VALUE hash; VALUE coder; - t_tmbo *this = DATA_PTR( self ); + t_tmbo *this = RTYPEDDATA_DATA( self ); int i_format = NUM2INT(format); struct pg_tmbo_oid_cache_entry *p_ce; @@ -269,7 +283,7 @@ pg_tmbo_rm_coder( VALUE self, VALUE format, VALUE oid ) static VALUE pg_tmbo_coders( VALUE self ) { - t_tmbo *this = DATA_PTR( self ); + t_tmbo *this = RTYPEDDATA_DATA( self ); return rb_ary_concat( rb_funcall(this->format[0].oid_to_coder, rb_intern("values"), 0), @@ -288,7 +302,7 @@ pg_tmbo_coders( VALUE self ) static VALUE pg_tmbo_max_rows_for_online_lookup_set( VALUE self, VALUE value ) { - t_tmbo *this = DATA_PTR( self ); + t_tmbo *this = RTYPEDDATA_DATA( self ); this->max_rows_for_online_lookup = NUM2INT(value); return value; } @@ -300,7 +314,7 @@ pg_tmbo_max_rows_for_online_lookup_set( VALUE self, VALUE value ) static VALUE pg_tmbo_max_rows_for_online_lookup_get( VALUE self ) { - t_tmbo *this = DATA_PTR( self ); + t_tmbo *this = RTYPEDDATA_DATA( self ); return INT2NUM(this->max_rows_for_online_lookup); } @@ -315,7 +329,7 @@ pg_tmbo_max_rows_for_online_lookup_get( VALUE self ) static VALUE pg_tmbo_build_column_map( VALUE self, VALUE result ) { - t_tmbo *this = DATA_PTR( self ); + t_tmbo *this = RTYPEDDATA_DATA( self ); if ( !rb_obj_is_kind_of(result, rb_cPGresult) ) { rb_raise( rb_eTypeError, "wrong argument type %s (expected kind of PG::Result)", diff --git a/ext/pg_type_map_in_ruby.c b/ext/pg_type_map_in_ruby.c index 78ccb3473..414304405 100644 --- a/ext/pg_type_map_in_ruby.c +++ b/ext/pg_type_map_in_ruby.c @@ -6,6 +6,20 @@ #include "pg.h" +static const rb_data_type_t pg_tmir_type = { + "PG::TypeMapInRuby", + { + (void (*)(void*))NULL, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + &pg_typemap_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + VALUE rb_cTypeMapInRuby; static VALUE s_id_fit_to_result; static VALUE s_id_fit_to_query; @@ -34,33 +48,37 @@ typedef struct { static VALUE pg_tmir_fit_to_result( VALUE self, VALUE result ) { - t_tmir *this = DATA_PTR( self ); + t_tmir *this = RTYPEDDATA_DATA( self ); t_typemap *default_tm; t_typemap *p_new_typemap; VALUE sub_typemap; VALUE new_typemap; if( rb_respond_to(self, s_id_fit_to_result) ){ + t_typemap *tm; + UNUSED(tm); new_typemap = rb_funcall( self, s_id_fit_to_result, 1, result ); if ( !rb_obj_is_kind_of(new_typemap, rb_cTypeMap) ) { + /* TypedData_Get_Struct() raises "wrong argument type", which is misleading, + * so we better raise our own message */ rb_raise( rb_eTypeError, "wrong return type from fit_to_result: %s expected kind of PG::TypeMap", rb_obj_classname( new_typemap ) ); } - Check_Type( new_typemap, T_DATA ); + TypedData_Get_Struct(new_typemap, t_typemap, &pg_typemap_type, tm); } else { new_typemap = self; } /* Ensure that the default type map fits equaly. */ - default_tm = DATA_PTR( this->typemap.default_typemap ); + default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); sub_typemap = default_tm->funcs.fit_to_result( this->typemap.default_typemap, result ); if( sub_typemap != this->typemap.default_typemap ){ new_typemap = rb_obj_dup( new_typemap ); } - p_new_typemap = DATA_PTR(new_typemap); + p_new_typemap = RTYPEDDATA_DATA(new_typemap); p_new_typemap->default_typemap = sub_typemap; return new_typemap; } @@ -95,8 +113,8 @@ pg_tmir_result_value( t_typemap *p_typemap, VALUE result, int tuple, int field ) static VALUE pg_tmir_typecast_result_value( VALUE self, VALUE result, VALUE tuple, VALUE field ) { - t_tmir *this = DATA_PTR( self ); - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_tmir *this = RTYPEDDATA_DATA( self ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); return default_tm->funcs.typecast_result_value( default_tm, result, NUM2INT(tuple), NUM2INT(field) ); } @@ -113,7 +131,7 @@ pg_tmir_typecast_result_value( VALUE self, VALUE result, VALUE tuple, VALUE fiel static VALUE pg_tmir_fit_to_query( VALUE self, VALUE params ) { - t_tmir *this = DATA_PTR( self ); + t_tmir *this = RTYPEDDATA_DATA( self ); t_typemap *default_tm; if( rb_respond_to(self, s_id_fit_to_query) ){ @@ -121,7 +139,7 @@ pg_tmir_fit_to_query( VALUE self, VALUE params ) } /* Ensure that the default type map fits equaly. */ - default_tm = DATA_PTR( this->typemap.default_typemap ); + default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); default_tm->funcs.fit_to_query( this->typemap.default_typemap, params ); return self; @@ -161,8 +179,8 @@ pg_tmir_query_param( t_typemap *p_typemap, VALUE param_value, int field ) static VALUE pg_tmir_typecast_query_param( VALUE self, VALUE param_value, VALUE field ) { - t_tmir *this = DATA_PTR( self ); - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_tmir *this = RTYPEDDATA_DATA( self ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); t_pg_coder *p_coder = default_tm->funcs.typecast_query_param( default_tm, param_value, NUM2INT(field) ); return p_coder ? p_coder->coder_obj : Qnil; @@ -186,7 +204,7 @@ static VALUE pg_tmir_fit_to_copy_get_dummy( VALUE self ){} static int pg_tmir_fit_to_copy_get( VALUE self ) { - t_tmir *this = DATA_PTR( self ); + t_tmir *this = RTYPEDDATA_DATA( self ); t_typemap *default_tm; VALUE num_columns = INT2NUM(0); @@ -199,7 +217,7 @@ pg_tmir_fit_to_copy_get( VALUE self ) rb_obj_classname( num_columns ) ); } /* Ensure that the default type map fits equaly. */ - default_tm = DATA_PTR( this->typemap.default_typemap ); + default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); default_tm->funcs.fit_to_copy_get( this->typemap.default_typemap ); return NUM2INT(num_columns);; @@ -239,8 +257,8 @@ pg_tmir_copy_get( t_typemap *p_typemap, VALUE field_str, int fieldno, int format static VALUE pg_tmir_typecast_copy_get( VALUE self, VALUE field_str, VALUE fieldno, VALUE format, VALUE enc ) { - t_tmir *this = DATA_PTR( self ); - t_typemap *default_tm = DATA_PTR( this->typemap.default_typemap ); + t_tmir *this = RTYPEDDATA_DATA( self ); + t_typemap *default_tm = RTYPEDDATA_DATA( this->typemap.default_typemap ); int enc_idx = rb_to_encoding_index( enc ); return default_tm->funcs.typecast_copy_get( default_tm, field_str, NUM2INT(fieldno), NUM2INT(format), enc_idx ); @@ -252,7 +270,7 @@ pg_tmir_s_allocate( VALUE klass ) t_tmir *this; VALUE self; - self = Data_Make_Struct( klass, t_tmir, NULL, -1, this ); + self = TypedData_Make_Struct( klass, t_tmir, &pg_tmir_type, this ); this->typemap.funcs.fit_to_result = pg_tmir_fit_to_result; this->typemap.funcs.fit_to_query = pg_tmir_fit_to_query; From 8cf705d6aee143af4ec04a67997d49206b6c0edd Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Wed, 5 Aug 2020 20:02:28 +0200 Subject: [PATCH 2/9] Convert PG::Coder derivations to TypedData objects --- ext/pg.h | 1 + ext/pg_coder.c | 88 ++++++++++++++++++++-------- ext/pg_connection.c | 34 +++++------ ext/pg_copy_coder.c | 30 +++++++--- ext/pg_record_coder.c | 22 +++++-- ext/pg_type_map_by_class.c | 8 +-- ext/pg_type_map_by_column.c | 6 +- ext/pg_type_map_by_mri_type.c | 10 +--- ext/pg_type_map_by_oid.c | 8 +-- ext/pg_type_map_in_ruby.c | 2 +- spec/pg/type_map_by_class_spec.rb | 2 +- spec/pg/type_map_by_column_spec.rb | 2 +- spec/pg/type_map_by_mri_type_spec.rb | 2 +- spec/pg/type_map_by_oid_spec.rb | 2 +- 14 files changed, 135 insertions(+), 82 deletions(-) diff --git a/ext/pg.h b/ext/pg.h index 61e9b0f28..7469bdb15 100644 --- a/ext/pg.h +++ b/ext/pg.h @@ -221,6 +221,7 @@ typedef struct { } t_tmbc; extern const rb_data_type_t pg_typemap_type; +extern const rb_data_type_t pg_coder_type; #include "gvl_wrappers.h" diff --git a/ext/pg_coder.c b/ext/pg_coder.c index 026fa2a58..5567ce2e2 100644 --- a/ext/pg_coder.c +++ b/ext/pg_coder.c @@ -26,11 +26,11 @@ pg_coder_allocate( VALUE klass ) void pg_coder_init_encoder( VALUE self ) { - t_pg_coder *this = DATA_PTR( self ); + t_pg_coder *this = RTYPEDDATA_DATA( self ); VALUE klass = rb_class_of(self); if( rb_const_defined( klass, s_id_CFUNC ) ){ VALUE cfunc = rb_const_get( klass, s_id_CFUNC ); - this->enc_func = DATA_PTR(cfunc); + this->enc_func = RTYPEDDATA_DATA(cfunc); } else { this->enc_func = NULL; } @@ -45,12 +45,12 @@ pg_coder_init_encoder( VALUE self ) void pg_coder_init_decoder( VALUE self ) { - t_pg_coder *this = DATA_PTR( self ); + t_pg_coder *this = RTYPEDDATA_DATA( self ); VALUE klass = rb_class_of(self); this->enc_func = NULL; if( rb_const_defined( klass, s_id_CFUNC ) ){ VALUE cfunc = rb_const_get( klass, s_id_CFUNC ); - this->dec_func = DATA_PTR(cfunc); + this->dec_func = RTYPEDDATA_DATA(cfunc); } else { this->dec_func = NULL; } @@ -73,20 +73,48 @@ pg_composite_coder_mark(t_pg_composite_coder *this) pg_coder_mark(&this->comp); } +const rb_data_type_t pg_coder_type = { + "PG::Coder", + { + (void (*)(void*))pg_coder_mark, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + 0, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + static VALUE pg_simple_encoder_allocate( VALUE klass ) { t_pg_coder *this; - VALUE self = Data_Make_Struct( klass, t_pg_coder, pg_coder_mark, -1, this ); + VALUE self = TypedData_Make_Struct( klass, t_pg_coder, &pg_coder_type, this ); pg_coder_init_encoder( self ); return self; } +static const rb_data_type_t pg_composite_coder_type = { + "PG::CompositeCoder", + { + (void (*)(void*))pg_composite_coder_mark, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + &pg_coder_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + static VALUE pg_composite_encoder_allocate( VALUE klass ) { t_pg_composite_coder *this; - VALUE self = Data_Make_Struct( klass, t_pg_composite_coder, pg_composite_coder_mark, -1, this ); + VALUE self = TypedData_Make_Struct( klass, t_pg_composite_coder, &pg_composite_coder_type, this ); pg_coder_init_encoder( self ); this->elem = NULL; this->needs_quotation = 1; @@ -99,7 +127,7 @@ static VALUE pg_simple_decoder_allocate( VALUE klass ) { t_pg_coder *this; - VALUE self = Data_Make_Struct( klass, t_pg_coder, pg_coder_mark, -1, this ); + VALUE self = TypedData_Make_Struct( klass, t_pg_coder, &pg_coder_type, this ); pg_coder_init_decoder( self ); return self; } @@ -108,7 +136,7 @@ static VALUE pg_composite_decoder_allocate( VALUE klass ) { t_pg_composite_coder *this; - VALUE self = Data_Make_Struct( klass, t_pg_composite_coder, pg_composite_coder_mark, -1, this ); + VALUE self = TypedData_Make_Struct( klass, t_pg_composite_coder, &pg_composite_coder_type, this ); pg_coder_init_decoder( self ); this->elem = NULL; this->needs_quotation = 1; @@ -135,7 +163,7 @@ pg_coder_encode(int argc, VALUE *argv, VALUE self) VALUE value; int len, len2; int enc_idx; - t_pg_coder *this = DATA_PTR(self); + t_pg_coder *this = RTYPEDDATA_DATA(self); if(argc < 1 || argc > 2){ rb_raise(rb_eArgError, "wrong number of arguments (%i for 1..2)", argc); @@ -192,7 +220,7 @@ pg_coder_decode(int argc, VALUE *argv, VALUE self) int tuple = -1; int field = -1; VALUE res; - t_pg_coder *this = DATA_PTR(self); + t_pg_coder *this = RTYPEDDATA_DATA(self); if(argc < 1 || argc > 3){ rb_raise(rb_eArgError, "wrong number of arguments (%i for 1..3)", argc); @@ -230,7 +258,7 @@ pg_coder_decode(int argc, VALUE *argv, VALUE self) static VALUE pg_coder_oid_set(VALUE self, VALUE oid) { - t_pg_coder *this = DATA_PTR(self); + t_pg_coder *this = RTYPEDDATA_DATA(self); this->oid = NUM2UINT(oid); return oid; } @@ -245,7 +273,7 @@ pg_coder_oid_set(VALUE self, VALUE oid) static VALUE pg_coder_oid_get(VALUE self) { - t_pg_coder *this = DATA_PTR(self); + t_pg_coder *this = RTYPEDDATA_DATA(self); return UINT2NUM(this->oid); } @@ -261,7 +289,7 @@ pg_coder_oid_get(VALUE self) static VALUE pg_coder_format_set(VALUE self, VALUE format) { - t_pg_coder *this = DATA_PTR(self); + t_pg_coder *this = RTYPEDDATA_DATA(self); this->format = NUM2INT(format); return format; } @@ -276,7 +304,7 @@ pg_coder_format_set(VALUE self, VALUE format) static VALUE pg_coder_format_get(VALUE self) { - t_pg_coder *this = DATA_PTR(self); + t_pg_coder *this = RTYPEDDATA_DATA(self); return INT2NUM(this->format); } @@ -292,7 +320,7 @@ pg_coder_format_get(VALUE self) static VALUE pg_coder_flags_set(VALUE self, VALUE flags) { - t_pg_coder *this = DATA_PTR(self); + t_pg_coder *this = RTYPEDDATA_DATA(self); this->flags = NUM2INT(flags); return flags; } @@ -306,7 +334,7 @@ pg_coder_flags_set(VALUE self, VALUE flags) static VALUE pg_coder_flags_get(VALUE self) { - t_pg_coder *this = DATA_PTR(self); + t_pg_coder *this = RTYPEDDATA_DATA(self); return INT2NUM(this->flags); } @@ -323,7 +351,7 @@ pg_coder_flags_get(VALUE self) static VALUE pg_coder_needs_quotation_set(VALUE self, VALUE needs_quotation) { - t_pg_composite_coder *this = DATA_PTR(self); + t_pg_composite_coder *this = RTYPEDDATA_DATA(self); this->needs_quotation = RTEST(needs_quotation); return needs_quotation; } @@ -338,7 +366,7 @@ pg_coder_needs_quotation_set(VALUE self, VALUE needs_quotation) static VALUE pg_coder_needs_quotation_get(VALUE self) { - t_pg_composite_coder *this = DATA_PTR(self); + t_pg_composite_coder *this = RTYPEDDATA_DATA(self); return this->needs_quotation ? Qtrue : Qfalse; } @@ -353,7 +381,7 @@ pg_coder_needs_quotation_get(VALUE self) static VALUE pg_coder_delimiter_set(VALUE self, VALUE delimiter) { - t_pg_composite_coder *this = DATA_PTR(self); + t_pg_composite_coder *this = RTYPEDDATA_DATA(self); StringValue(delimiter); if(RSTRING_LEN(delimiter) != 1) rb_raise( rb_eArgError, "delimiter size must be one byte"); @@ -370,7 +398,7 @@ pg_coder_delimiter_set(VALUE self, VALUE delimiter) static VALUE pg_coder_delimiter_get(VALUE self) { - t_pg_composite_coder *this = DATA_PTR(self); + t_pg_composite_coder *this = RTYPEDDATA_DATA(self); return rb_str_new(&this->delimiter, 1); } @@ -386,12 +414,12 @@ pg_coder_delimiter_get(VALUE self) static VALUE pg_coder_elements_type_set(VALUE self, VALUE elem_type) { - t_pg_composite_coder *this = DATA_PTR( self ); + t_pg_composite_coder *this = RTYPEDDATA_DATA( self ); if ( NIL_P(elem_type) ){ this->elem = NULL; } else if ( rb_obj_is_kind_of(elem_type, rb_cPG_Coder) ){ - this->elem = DATA_PTR( elem_type ); + this->elem = RTYPEDDATA_DATA( elem_type ); } else { rb_raise( rb_eTypeError, "wrong elements type %s (expected some kind of PG::Coder)", rb_obj_classname( elem_type ) ); @@ -401,10 +429,24 @@ pg_coder_elements_type_set(VALUE self, VALUE elem_type) return elem_type; } +static const rb_data_type_t pg_coder_cfunc_type = { + "PG::Coder::CFUNC", + { + (void (*)(void*))NULL, + (void (*)(void*))NULL, + (size_t (*)(const void *))NULL, + }, + 0, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + void pg_define_coder( const char *name, void *func, VALUE base_klass, VALUE nsp ) { - VALUE cfunc_obj = Data_Wrap_Struct( rb_cObject, NULL, NULL, func ); + VALUE cfunc_obj = TypedData_Wrap_Struct( rb_cObject, &pg_coder_cfunc_type, func ); VALUE coder_klass = rb_define_class_under( nsp, name, base_klass ); if( nsp==rb_mPG_BinaryEncoder || nsp==rb_mPG_BinaryDecoder ) rb_include_module( coder_klass, rb_mPG_BinaryFormatting ); diff --git a/ext/pg_connection.c b/ext/pg_connection.c index e47c11609..8442fcb64 100644 --- a/ext/pg_connection.c +++ b/ext/pg_connection.c @@ -2466,13 +2466,11 @@ pgconn_put_copy_data(int argc, VALUE *argv, VALUE self) if( NIL_P(this->encoder_for_put_copy_data) ){ buffer = value; } else { - p_coder = DATA_PTR( this->encoder_for_put_copy_data ); + p_coder = RTYPEDDATA_DATA( this->encoder_for_put_copy_data ); } - } else if( rb_obj_is_kind_of(encoder, rb_cPG_Coder) ) { - Data_Get_Struct( encoder, t_pg_coder, p_coder ); } else { - rb_raise( rb_eTypeError, "wrong encoder type %s (expected some kind of PG::Coder)", - rb_obj_classname( encoder ) ); + /* Check argument type and use argument encoder */ + TypedData_Get_Struct(encoder, t_pg_coder, &pg_coder_type, p_coder); } if( p_coder ){ @@ -2578,13 +2576,11 @@ pgconn_get_copy_data(int argc, VALUE *argv, VALUE self ) if( NIL_P(decoder) ){ if( !NIL_P(this->decoder_for_get_copy_data) ){ - p_coder = DATA_PTR( this->decoder_for_get_copy_data ); + p_coder = RTYPEDDATA_DATA( this->decoder_for_get_copy_data ); } - } else if( rb_obj_is_kind_of(decoder, rb_cPG_Coder) ) { - Data_Get_Struct( decoder, t_pg_coder, p_coder ); } else { - rb_raise( rb_eTypeError, "wrong decoder type %s (expected some kind of PG::Coder)", - rb_obj_classname( decoder ) ); + /* Check argument type and use argument decoder */ + TypedData_Get_Struct(decoder, t_pg_coder, &pg_coder_type, p_coder); } ret = gvl_PQgetCopyData(this->pgconn, &buffer, RTEST(async_in)); @@ -3998,11 +3994,10 @@ pgconn_encoder_for_put_copy_data_set(VALUE self, VALUE encoder) t_pg_connection *this = pg_get_connection( self ); if( encoder != Qnil ){ - if ( !rb_obj_is_kind_of(encoder, rb_cPG_Coder) ) { - rb_raise( rb_eTypeError, "wrong argument type %s (expected kind of PG::Coder)", - rb_obj_classname( encoder ) ); - } - Check_Type(encoder, T_DATA); + t_pg_coder *co; + UNUSED(co); + /* Check argument type */ + TypedData_Get_Struct(encoder, t_pg_coder, &pg_coder_type, co); } this->encoder_for_put_copy_data = encoder; @@ -4047,11 +4042,10 @@ pgconn_decoder_for_get_copy_data_set(VALUE self, VALUE decoder) t_pg_connection *this = pg_get_connection( self ); if( decoder != Qnil ){ - if ( !rb_obj_is_kind_of(decoder, rb_cPG_Coder) ) { - rb_raise( rb_eTypeError, "wrong argument type %s (expected kind of PG::Coder)", - rb_obj_classname( decoder ) ); - } - Check_Type(decoder, T_DATA); + t_pg_coder *co; + UNUSED(co); + /* Check argument type */ + TypedData_Get_Struct(decoder, t_pg_coder, &pg_coder_type, co); } this->decoder_for_get_copy_data = decoder; diff --git a/ext/pg_copy_coder.c b/ext/pg_copy_coder.c index 19222e0ca..899a7b51f 100644 --- a/ext/pg_copy_coder.c +++ b/ext/pg_copy_coder.c @@ -28,11 +28,25 @@ pg_copycoder_mark( t_pg_copycoder *this ) rb_gc_mark(this->null_string); } +static const rb_data_type_t pg_copycoder_type = { + "PG::CopyCoder", + { + (void (*)(void*))pg_copycoder_mark, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + &pg_coder_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + static VALUE pg_copycoder_encoder_allocate( VALUE klass ) { t_pg_copycoder *this; - VALUE self = Data_Make_Struct( klass, t_pg_copycoder, pg_copycoder_mark, -1, this ); + VALUE self = TypedData_Make_Struct( klass, t_pg_copycoder, &pg_copycoder_type, this ); pg_coder_init_encoder( self ); this->typemap = pg_typemap_all_strings; this->delimiter = '\t'; @@ -44,7 +58,7 @@ static VALUE pg_copycoder_decoder_allocate( VALUE klass ) { t_pg_copycoder *this; - VALUE self = Data_Make_Struct( klass, t_pg_copycoder, pg_copycoder_mark, -1, this ); + VALUE self = TypedData_Make_Struct( klass, t_pg_copycoder, &pg_copycoder_type, this ); pg_coder_init_decoder( self ); this->typemap = pg_typemap_all_strings; this->delimiter = '\t'; @@ -63,7 +77,7 @@ pg_copycoder_decoder_allocate( VALUE klass ) static VALUE pg_copycoder_delimiter_set(VALUE self, VALUE delimiter) { - t_pg_copycoder *this = DATA_PTR(self); + t_pg_copycoder *this = RTYPEDDATA_DATA(self); StringValue(delimiter); if(RSTRING_LEN(delimiter) != 1) rb_raise( rb_eArgError, "delimiter size must be one byte"); @@ -80,7 +94,7 @@ pg_copycoder_delimiter_set(VALUE self, VALUE delimiter) static VALUE pg_copycoder_delimiter_get(VALUE self) { - t_pg_copycoder *this = DATA_PTR(self); + t_pg_copycoder *this = RTYPEDDATA_DATA(self); return rb_str_new(&this->delimiter, 1); } @@ -93,7 +107,7 @@ pg_copycoder_delimiter_get(VALUE self) static VALUE pg_copycoder_null_string_set(VALUE self, VALUE null_string) { - t_pg_copycoder *this = DATA_PTR(self); + t_pg_copycoder *this = RTYPEDDATA_DATA(self); StringValue(null_string); this->null_string = null_string; return null_string; @@ -105,7 +119,7 @@ pg_copycoder_null_string_set(VALUE self, VALUE null_string) static VALUE pg_copycoder_null_string_get(VALUE self) { - t_pg_copycoder *this = DATA_PTR(self); + t_pg_copycoder *this = RTYPEDDATA_DATA(self); return this->null_string; } @@ -123,7 +137,7 @@ pg_copycoder_null_string_get(VALUE self) static VALUE pg_copycoder_type_map_set(VALUE self, VALUE type_map) { - t_pg_copycoder *this = DATA_PTR( self ); + t_pg_copycoder *this = RTYPEDDATA_DATA( self ); if ( !rb_obj_is_kind_of(type_map, rb_cTypeMap) ){ rb_raise( rb_eTypeError, "wrong elements type %s (expected some kind of PG::TypeMap)", @@ -143,7 +157,7 @@ pg_copycoder_type_map_set(VALUE self, VALUE type_map) static VALUE pg_copycoder_type_map_get(VALUE self) { - t_pg_copycoder *this = DATA_PTR( self ); + t_pg_copycoder *this = RTYPEDDATA_DATA( self ); return this->typemap; } diff --git a/ext/pg_record_coder.c b/ext/pg_record_coder.c index a718ee11b..48d583fff 100644 --- a/ext/pg_record_coder.c +++ b/ext/pg_record_coder.c @@ -22,11 +22,25 @@ pg_recordcoder_mark( t_pg_recordcoder *this ) rb_gc_mark(this->typemap); } +static const rb_data_type_t pg_recordcoder_type = { + "PG::RecordCoder", + { + (void (*)(void*))pg_recordcoder_mark, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + &pg_coder_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + static VALUE pg_recordcoder_encoder_allocate( VALUE klass ) { t_pg_recordcoder *this; - VALUE self = Data_Make_Struct( klass, t_pg_recordcoder, pg_recordcoder_mark, -1, this ); + VALUE self = TypedData_Make_Struct( klass, t_pg_recordcoder, &pg_recordcoder_type, this ); pg_coder_init_encoder( self ); this->typemap = pg_typemap_all_strings; return self; @@ -36,7 +50,7 @@ static VALUE pg_recordcoder_decoder_allocate( VALUE klass ) { t_pg_recordcoder *this; - VALUE self = Data_Make_Struct( klass, t_pg_recordcoder, pg_recordcoder_mark, -1, this ); + VALUE self = TypedData_Make_Struct( klass, t_pg_recordcoder, &pg_recordcoder_type, this ); pg_coder_init_decoder( self ); this->typemap = pg_typemap_all_strings; return self; @@ -56,7 +70,7 @@ pg_recordcoder_decoder_allocate( VALUE klass ) static VALUE pg_recordcoder_type_map_set(VALUE self, VALUE type_map) { - t_pg_recordcoder *this = DATA_PTR( self ); + t_pg_recordcoder *this = RTYPEDDATA_DATA( self ); if ( !rb_obj_is_kind_of(type_map, rb_cTypeMap) ){ rb_raise( rb_eTypeError, "wrong elements type %s (expected some kind of PG::TypeMap)", @@ -76,7 +90,7 @@ pg_recordcoder_type_map_set(VALUE self, VALUE type_map) static VALUE pg_recordcoder_type_map_get(VALUE self) { - t_pg_recordcoder *this = DATA_PTR( self ); + t_pg_recordcoder *this = RTYPEDDATA_DATA( self ); return this->typemap; } diff --git a/ext/pg_type_map_by_class.c b/ext/pg_type_map_by_class.c index eb0b666ca..4981e8312 100644 --- a/ext/pg_type_map_by_class.c +++ b/ext/pg_type_map_by_class.c @@ -62,7 +62,7 @@ pg_tmbk_lookup_klass(t_tmbk *this, VALUE klass, VALUE param_value) if(NIL_P(obj)){ p_coder = NULL; }else if(rb_obj_is_kind_of(obj, rb_cPG_Coder)){ - Data_Get_Struct(obj, t_pg_coder, p_coder); + TypedData_Get_Struct(obj, t_pg_coder, &pg_coder_type, p_coder); }else{ if( RB_TYPE_P(obj, T_SYMBOL) ){ /* A Symbol: Call the method with this name. */ @@ -74,11 +74,9 @@ pg_tmbk_lookup_klass(t_tmbk *this, VALUE klass, VALUE param_value) if( NIL_P(obj) ){ p_coder = NULL; - }else if( rb_obj_is_kind_of(obj, rb_cPG_Coder) ){ - Data_Get_Struct(obj, t_pg_coder, p_coder); }else{ - rb_raise(rb_eTypeError, "argument has invalid type %s (should be nil or some kind of PG::Coder)", - rb_obj_classname( obj )); + /* Check retrieved coder type */ + TypedData_Get_Struct(obj, t_pg_coder, &pg_coder_type, p_coder); } /* We can not cache coders retrieved by ruby code, because we can not anticipate diff --git a/ext/pg_type_map_by_column.c b/ext/pg_type_map_by_column.c index b7262deb3..7114943d6 100644 --- a/ext/pg_type_map_by_column.c +++ b/ext/pg_type_map_by_column.c @@ -255,11 +255,9 @@ pg_tmbc_init(VALUE self, VALUE conv_ary) if( obj == Qnil ){ /* no type cast */ this->convs[i].cconv = NULL; - } else if( rb_obj_is_kind_of(obj, rb_cPG_Coder) ){ - Data_Get_Struct(obj, t_pg_coder, this->convs[i].cconv); } else { - rb_raise(rb_eArgError, "argument %d has invalid type %s (should be nil or some kind of PG::Coder)", - i+1, rb_obj_classname( obj )); + /* Check argument type and store the coder pointer */ + TypedData_Get_Struct(obj, t_pg_coder, &pg_coder_type, this->convs[i].cconv); } } diff --git a/ext/pg_type_map_by_mri_type.c b/ext/pg_type_map_by_mri_type.c index 258e74c6b..d1a1cf4dc 100644 --- a/ext/pg_type_map_by_mri_type.c +++ b/ext/pg_type_map_by_mri_type.c @@ -71,12 +71,8 @@ pg_tmbmt_typecast_query_param( t_typemap *p_typemap, VALUE param_value, int fiel obj = rb_funcall(ask_for_coder, rb_intern("call"), 1, param_value); - if( rb_obj_is_kind_of(obj, rb_cPG_Coder) ){ - Data_Get_Struct(obj, t_pg_coder, p_coder); - }else{ - rb_raise(rb_eTypeError, "argument %d has invalid type %s (should be nil or some kind of PG::Coder)", - field+1, rb_obj_classname( obj )); - } + /* Check argument type and store the coder pointer */ + TypedData_Get_Struct(obj, t_pg_coder, &pg_coder_type, p_coder); } if( !p_coder ){ @@ -154,7 +150,7 @@ pg_tmbmt_s_allocate( VALUE klass ) this->coders.coder_##type = NULL; \ this->coders.ask_##type = Qnil; \ }else if(rb_obj_is_kind_of(coder, rb_cPG_Coder)){ \ - Data_Get_Struct(coder, t_pg_coder, this->coders.coder_##type); \ + TypedData_Get_Struct(coder, t_pg_coder, &pg_coder_type, this->coders.coder_##type); \ this->coders.ask_##type = Qnil; \ }else if(RB_TYPE_P(coder, T_SYMBOL)){ \ this->coders.coder_##type = NULL; \ diff --git a/ext/pg_type_map_by_oid.c b/ext/pg_type_map_by_oid.c index 06fc2b225..e7985d4ed 100644 --- a/ext/pg_type_map_by_oid.c +++ b/ext/pg_type_map_by_oid.c @@ -46,7 +46,7 @@ pg_tmbo_lookup_oid(t_tmbo *this, int format, Oid oid) } else { VALUE obj = rb_hash_lookup( this->format[format].oid_to_coder, UINT2NUM( oid )); /* obj must be nil or some kind of PG::Coder, this is checked at insertion */ - conv = NIL_P(obj) ? NULL : DATA_PTR(obj); + conv = NIL_P(obj) ? NULL : RTYPEDDATA_DATA(obj); /* Write the retrieved coder to the cache */ p_ce->oid = oid; p_ce->p_coder = conv; @@ -223,11 +223,7 @@ pg_tmbo_add_coder( VALUE self, VALUE coder ) t_pg_coder *p_coder; struct pg_tmbo_oid_cache_entry *p_ce; - if( !rb_obj_is_kind_of(coder, rb_cPG_Coder) ) - rb_raise(rb_eArgError, "invalid type %s (should be some kind of PG::Coder)", - rb_obj_classname( coder )); - - Data_Get_Struct(coder, t_pg_coder, p_coder); + TypedData_Get_Struct(coder, t_pg_coder, &pg_coder_type, p_coder); if( p_coder->format < 0 || p_coder->format > 1 ) rb_raise(rb_eArgError, "invalid format code %d", p_coder->format); diff --git a/ext/pg_type_map_in_ruby.c b/ext/pg_type_map_in_ruby.c index 414304405..1bc99a07b 100644 --- a/ext/pg_type_map_in_ruby.c +++ b/ext/pg_type_map_in_ruby.c @@ -155,7 +155,7 @@ pg_tmir_query_param( t_typemap *p_typemap, VALUE param_value, int field ) if ( NIL_P(coder) ){ return NULL; } else if( rb_obj_is_kind_of(coder, rb_cPG_Coder) ) { - return DATA_PTR(coder); + return RTYPEDDATA_DATA(coder); } else { rb_raise( rb_eTypeError, "wrong return type from typecast_query_param: %s expected nil or kind of PG::Coder", rb_obj_classname( coder ) ); diff --git a/spec/pg/type_map_by_class_spec.rb b/spec/pg/type_map_by_class_spec.rb index 2efa78746..a55b22247 100644 --- a/spec/pg/type_map_by_class_spec.rb +++ b/spec/pg/type_map_by_class_spec.rb @@ -126,7 +126,7 @@ def array_type_map_for(value) it "should raise TypeError with derived type map" do expect{ @conn.exec_params( "SELECT $1", [raise_class.new], 0, derived_tm ) - }.to raise_error(TypeError, /invalid type Regexp/) + }.to raise_error(TypeError, /wrong argument type Regexp/) end it "should raise error on invalid coder object" do diff --git a/spec/pg/type_map_by_column_spec.rb b/spec/pg/type_map_by_column_spec.rb index 810f0ce9c..f242fb4a2 100644 --- a/spec/pg/type_map_by_column_spec.rb +++ b/spec/pg/type_map_by_column_spec.rb @@ -159,7 +159,7 @@ def decode(res, tuple, field) it "should raise an error for invalid params" do expect{ PG::TypeMapByColumn.new( :WrongType ) }.to raise_error(TypeError, /wrong argument type/) - expect{ PG::TypeMapByColumn.new( [123] ) }.to raise_error(ArgumentError, /invalid/) + expect{ PG::TypeMapByColumn.new( [123] ) }.to raise_error(TypeError, /wrong argument type (Integer|Fixnum)/) end it "shouldn't allow result mappings with different number of fields" do diff --git a/spec/pg/type_map_by_mri_type_spec.rb b/spec/pg/type_map_by_mri_type_spec.rb index 22e989a85..ab2e9b7b3 100644 --- a/spec/pg/type_map_by_mri_type_spec.rb +++ b/spec/pg/type_map_by_mri_type_spec.rb @@ -130,7 +130,7 @@ def array_type_map_for(value) it "should raise TypeError with derived type map" do expect{ @conn.exec_params( "SELECT $1", [//], 0, derived_tm ) - }.to raise_error(TypeError, /argument 1/) + }.to raise_error(TypeError, /wrong argument type/) end end diff --git a/spec/pg/type_map_by_oid_spec.rb b/spec/pg/type_map_by_oid_spec.rb index 4a31ba95b..0a255fd5b 100644 --- a/spec/pg/type_map_by_oid_spec.rb +++ b/spec/pg/type_map_by_oid_spec.rb @@ -66,7 +66,7 @@ def decode(*v) end it "should check coder type when adding coders" do - expect{ tm.add_coder :dummy }.to raise_error(ArgumentError) + expect{ tm.add_coder :dummy }.to raise_error(TypeError) end it "should allow reading and writing max_rows_for_online_lookup" do From 3e8d34356ff59b23f01b63a6b590d30c8785286d Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Wed, 5 Aug 2020 22:26:10 +0200 Subject: [PATCH 3/9] Convert PG::Connection and it's temporary objects to TypedData objects All T_DATA objects of ruby-pg are based on TypedData_Struct now. So we're no longer using deprecated non-typed objects. --- ext/pg_connection.c | 58 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/ext/pg_connection.c b/ext/pg_connection.c index 8442fcb64..86f3fbd55 100644 --- a/ext/pg_connection.c +++ b/ext/pg_connection.c @@ -21,6 +21,7 @@ static PQnoticeProcessor default_notice_processor = NULL; static VALUE pgconn_finish( VALUE ); static VALUE pgconn_set_default_encoding( VALUE self ); static void pgconn_set_internal_encoding_index( VALUE ); +static const rb_data_type_t pg_connection_type; /* * Global functions @@ -33,7 +34,7 @@ t_pg_connection * pg_get_connection( VALUE self ) { t_pg_connection *this; - Data_Get_Struct( self, t_pg_connection, this); + TypedData_Get_Struct( self, t_pg_connection, &pg_connection_type, this); return this; } @@ -46,7 +47,7 @@ static t_pg_connection * pg_get_connection_safe( VALUE self ) { t_pg_connection *this; - Data_Get_Struct( self, t_pg_connection, this); + TypedData_Get_Struct( self, t_pg_connection, &pg_connection_type, this); if ( !this->pgconn ) rb_raise( rb_eConnectionBad, "connection is closed" ); @@ -65,7 +66,7 @@ PGconn * pg_get_pgconn( VALUE self ) { t_pg_connection *this; - Data_Get_Struct( self, t_pg_connection, this); + TypedData_Get_Struct( self, t_pg_connection, &pg_connection_type, this); if ( !this->pgconn ) rb_raise( rb_eConnectionBad, "connection is closed" ); @@ -174,6 +175,20 @@ pgconn_gc_free( t_pg_connection *this ) xfree(this); } +static const rb_data_type_t pg_connection_type = { + "PG::Connection", + { + (void (*)(void*))pgconn_gc_mark, + (void (*)(void*))pgconn_gc_free, + (size_t (*)(const void *))NULL, + }, + 0, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + /************************************************************************** * Class Methods @@ -189,7 +204,7 @@ static VALUE pgconn_s_allocate( VALUE klass ) { t_pg_connection *this; - VALUE self = Data_Make_Struct( klass, t_pg_connection, pgconn_gc_mark, pgconn_gc_free, this ); + VALUE self = TypedData_Make_Struct( klass, t_pg_connection, &pg_connection_type, this ); this->pgconn = NULL; this->socket_io = Qnil; @@ -1060,6 +1075,20 @@ free_typecast_heap_chain(struct linked_typecast_data *chain_entry) } } +static const rb_data_type_t pg_typecast_buffer_type = { + "PG::Connection typecast buffer chain", + { + (void (*)(void*))NULL, + (void (*)(void*))free_typecast_heap_chain, + (size_t (*)(const void *))NULL, + }, + 0, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + static char * alloc_typecast_buf( VALUE *typecast_heap_chain, int len ) { @@ -1070,17 +1099,30 @@ alloc_typecast_buf( VALUE *typecast_heap_chain, int len ) /* Did we already wrap a memory chain per T_DATA object? */ if( NIL_P( *typecast_heap_chain ) ){ /* Leave free'ing of the buffer chain to the GC, when paramsData has left the stack */ - *typecast_heap_chain = Data_Wrap_Struct( rb_cObject, NULL, free_typecast_heap_chain, allocated ); + *typecast_heap_chain = TypedData_Wrap_Struct( rb_cObject, &pg_typecast_buffer_type, allocated ); allocated->next = NULL; } else { /* Append to the chain */ - allocated->next = DATA_PTR( *typecast_heap_chain ); - DATA_PTR( *typecast_heap_chain ) = allocated; + allocated->next = RTYPEDDATA_DATA( *typecast_heap_chain ); + RTYPEDDATA_DATA( *typecast_heap_chain ) = allocated; } return &allocated->data[0]; } +static const rb_data_type_t pg_query_heap_pool_type = { + "PG::Connection query heap pool", + { + (void (*)(void*))NULL, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + }, + 0, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; static int alloc_query_params(struct query_params_data *paramsData) @@ -1114,7 +1156,7 @@ alloc_query_params(struct query_params_data *paramsData) /* Allocate one combined memory pool for all possible function parameters */ memory_pool = (char*)xmalloc( required_pool_size ); /* Leave free'ing of the buffer to the GC, when paramsData has left the stack */ - paramsData->heap_pool = Data_Wrap_Struct( rb_cObject, NULL, -1, memory_pool ); + paramsData->heap_pool = TypedData_Wrap_Struct( rb_cObject, &pg_query_heap_pool_type, memory_pool ); required_pool_size = 0; }else{ /* Use stack memory for function parameters */ From df958adf3b7712139a3647bbd7e8ecccc05928cc Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Sun, 9 Aug 2020 15:04:46 +0200 Subject: [PATCH 4/9] Make PG::Coder and derivations friendly to GC.compact This way the VALUE references may be relocated. Since self is always referenced indirectly, marking simple coders can be omitted entirely. This partly reverts 7c1756f953e0c405fc0bf987d046c416e5bb061c which was introduced as a simple fix for GC.compact compatbility. --- ext/extconf.rb | 1 + ext/pg.h | 11 ++++++++++- ext/pg_coder.c | 14 ++++++++------ ext/pg_copy_coder.c | 14 +++++++++++--- ext/pg_record_coder.c | 11 +++++++++-- 5 files changed, 39 insertions(+), 12 deletions(-) diff --git a/ext/extconf.rb b/ext/extconf.rb index 8bc1163bc..beced6e79 100755 --- a/ext/extconf.rb +++ b/ext/extconf.rb @@ -80,6 +80,7 @@ have_func 'PQresultMemorySize' # since PostgreSQL-12 have_func 'timegm' have_func 'rb_gc_adjust_memory_usage' # since ruby-2.4 +have_func 'rb_gc_mark_movable' # since ruby-2.7 # unistd.h confilicts with ruby/win32.h when cross compiling for win32 and ruby 1.9.1 have_header 'unistd.h' diff --git a/ext/pg.h b/ext/pg.h index 7469bdb15..20193476b 100644 --- a/ext/pg.h +++ b/ext/pg.h @@ -78,6 +78,15 @@ typedef long suseconds_t; #define RARRAY_AREF(a, i) (RARRAY_PTR(a)[i]) #endif +#ifdef HAVE_RB_GC_MARK_MOVABLE +#define pg_compact_callback(x) ((void (*)(void*))(x)) +#define pg_gc_location(x) x = rb_gc_location(x) +#else +#define rb_gc_mark_movable(x) rb_gc_mark(x) +#define pg_compact_callback(x) {(x)} +#define pg_gc_location(x) UNUSED(x) +#endif + #define PG_ENC_IDX_BITS 28 /* The data behind each PG::Connection object */ @@ -306,7 +315,7 @@ VALUE pg_obj_to_i _(( VALUE )); VALUE pg_tmbc_allocate _(( void )); void pg_coder_init_encoder _(( VALUE )); void pg_coder_init_decoder _(( VALUE )); -void pg_coder_mark _(( t_pg_coder * )); +void pg_coder_compact _(( t_pg_coder * )); char *pg_rb_str_ensure_capa _(( VALUE, long, char *, char ** )); #define PG_RB_STR_ENSURE_CAPA( str, expand_len, curr_ptr, end_ptr ) \ diff --git a/ext/pg_coder.c b/ext/pg_coder.c index 5567ce2e2..d52fba673 100644 --- a/ext/pg_coder.c +++ b/ext/pg_coder.c @@ -62,23 +62,24 @@ pg_coder_init_decoder( VALUE self ) } void -pg_coder_mark(t_pg_coder *this) +pg_coder_compact(t_pg_coder *this) { - rb_gc_mark(this->coder_obj); + pg_gc_location(this->coder_obj); } static void -pg_composite_coder_mark(t_pg_composite_coder *this) +pg_composite_coder_compact(t_pg_composite_coder *this) { - pg_coder_mark(&this->comp); + pg_coder_compact(&this->comp); } const rb_data_type_t pg_coder_type = { "PG::Coder", { - (void (*)(void*))pg_coder_mark, + (void (*)(void*))NULL, (void (*)(void*))-1, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_coder_compact), }, 0, 0, @@ -99,9 +100,10 @@ pg_simple_encoder_allocate( VALUE klass ) static const rb_data_type_t pg_composite_coder_type = { "PG::CompositeCoder", { - (void (*)(void*))pg_composite_coder_mark, + (void (*)(void*))NULL, (void (*)(void*))-1, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_composite_coder_compact), }, &pg_coder_type, 0, diff --git a/ext/pg_copy_coder.c b/ext/pg_copy_coder.c index 899a7b51f..44593eb95 100644 --- a/ext/pg_copy_coder.c +++ b/ext/pg_copy_coder.c @@ -23,9 +23,16 @@ typedef struct { static void pg_copycoder_mark( t_pg_copycoder *this ) { - pg_coder_mark(&this->comp); - rb_gc_mark(this->typemap); - rb_gc_mark(this->null_string); + rb_gc_mark_movable(this->typemap); + rb_gc_mark_movable(this->null_string); +} + +static void +pg_copycoder_compact( t_pg_copycoder *this ) +{ + pg_coder_compact(&this->comp); + pg_gc_location(this->typemap); + pg_gc_location(this->null_string); } static const rb_data_type_t pg_copycoder_type = { @@ -34,6 +41,7 @@ static const rb_data_type_t pg_copycoder_type = { (void (*)(void*))pg_copycoder_mark, (void (*)(void*))-1, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_copycoder_compact), }, &pg_coder_type, 0, diff --git a/ext/pg_record_coder.c b/ext/pg_record_coder.c index 48d583fff..b6696a4df 100644 --- a/ext/pg_record_coder.c +++ b/ext/pg_record_coder.c @@ -18,8 +18,14 @@ typedef struct { static void pg_recordcoder_mark( t_pg_recordcoder *this ) { - pg_coder_mark(&this->comp); - rb_gc_mark(this->typemap); + rb_gc_mark_movable(this->typemap); +} + +static void +pg_recordcoder_compact( t_pg_recordcoder *this ) +{ + pg_coder_compact(&this->comp); + pg_gc_location(this->typemap); } static const rb_data_type_t pg_recordcoder_type = { @@ -28,6 +34,7 @@ static const rb_data_type_t pg_recordcoder_type = { (void (*)(void*))pg_recordcoder_mark, (void (*)(void*))-1, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_recordcoder_compact), }, &pg_coder_type, 0, From a817bff3b2ee4fe38a4373992dabfe7ee4b44538 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Sun, 9 Aug 2020 17:15:17 +0200 Subject: [PATCH 5/9] Make PG::TypeMap and derivations friendly to GC.compact This way the VALUE references may be relocated. This partly reverts 7c1756f953e0c405fc0bf987d046c416e5bb061c which was introduced as a simple fix for GC.compact compatbility. --- ext/pg.h | 2 ++ ext/pg_type_map.c | 15 ++++++++++++++- ext/pg_type_map_all_strings.c | 3 ++- ext/pg_type_map_by_class.c | 21 ++++++++++++++------ ext/pg_type_map_by_column.c | 21 ++++++++++++++++++-- ext/pg_type_map_by_mri_type.c | 18 +++++++++++++++--- ext/pg_type_map_by_oid.c | 16 ++++++++++++++-- ext/pg_type_map_in_ruby.c | 36 +++++++++++++++++++++-------------- 8 files changed, 103 insertions(+), 29 deletions(-) diff --git a/ext/pg.h b/ext/pg.h index 20193476b..27fc96aed 100644 --- a/ext/pg.h +++ b/ext/pg.h @@ -335,6 +335,8 @@ int pg_typemap_fit_to_copy_get _(( VALUE )); VALUE pg_typemap_result_value _(( t_typemap *, VALUE, int, int )); t_pg_coder *pg_typemap_typecast_query_param _(( t_typemap *, VALUE, int )); VALUE pg_typemap_typecast_copy_get _(( t_typemap *, VALUE, int, int, int )); +void pg_typemap_mark _(( t_typemap * )); +void pg_typemap_compact _(( t_typemap * )); PGconn *pg_get_pgconn _(( VALUE )); t_pg_connection *pg_get_connection _(( VALUE )); diff --git a/ext/pg_type_map.c b/ext/pg_type_map.c index f9ee05459..757d25cbb 100644 --- a/ext/pg_type_map.c +++ b/ext/pg_type_map.c @@ -6,12 +6,25 @@ #include "pg.h" +void +pg_typemap_mark( t_typemap *this ) +{ + rb_gc_mark_movable(this->default_typemap); +} + +void +pg_typemap_compact( t_typemap *this ) +{ + pg_gc_location(this->default_typemap); +} + const rb_data_type_t pg_typemap_type = { "PG::TypeMap", { - (void (*)(void*))NULL, + (void (*)(void*))pg_typemap_mark, (void (*)(void*))-1, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_typemap_compact), }, 0, 0, diff --git a/ext/pg_type_map_all_strings.c b/ext/pg_type_map_all_strings.c index 9a721708f..c1721ad4b 100644 --- a/ext/pg_type_map_all_strings.c +++ b/ext/pg_type_map_all_strings.c @@ -11,9 +11,10 @@ static const rb_data_type_t pg_tmas_type = { "PG::TypeMapAllStrings", { - (void (*)(void*))NULL, + (void (*)(void*))pg_typemap_mark, (void (*)(void*))-1, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_typemap_compact), }, &pg_typemap_type, 0, diff --git a/ext/pg_type_map_by_class.c b/ext/pg_type_map_by_class.c index 4981e8312..498141cb9 100644 --- a/ext/pg_type_map_by_class.c +++ b/ext/pg_type_map_by_class.c @@ -121,12 +121,20 @@ pg_tmbk_fit_to_query( VALUE self, VALUE params ) static void pg_tmbk_mark( t_tmbk *this ) { - rb_gc_mark(this->typemap.default_typemap); - rb_gc_mark(this->klass_to_coder); - rb_gc_mark(this->self); - /* Clear the cache, to be safe from changes of klass VALUE by GC.compact. - * TODO: Move cache clearing to compactation callback provided by Ruby-2.7+. - */ + pg_typemap_mark(&this->typemap); + rb_gc_mark_movable(this->klass_to_coder); +} + +static void +pg_tmbk_compact(void *ptr) +{ + t_tmbk *this = (t_tmbk *)ptr; + + pg_typemap_compact(&this->typemap); + pg_gc_location(this->klass_to_coder); + pg_gc_location(this->self); + + /* Clear the cache, to be safe from changes of klass VALUE by GC.compact. */ memset(&this->cache_row, 0, sizeof(this->cache_row)); } @@ -136,6 +144,7 @@ static const rb_data_type_t pg_tmbk_type = { (void (*)(void*))pg_tmbk_mark, (void (*)(void*))-1, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_tmbk_compact), }, &pg_typemap_type, 0, diff --git a/ext/pg_type_map_by_column.c b/ext/pg_type_map_by_column.c index 7114943d6..3bbe1db7c 100644 --- a/ext/pg_type_map_by_column.c +++ b/ext/pg_type_map_by_column.c @@ -178,11 +178,27 @@ pg_tmbc_mark( t_tmbc *this ) /* allocated but not initialized ? */ if( this == (t_tmbc *)&pg_typemap_funcs ) return; - rb_gc_mark(this->typemap.default_typemap); + pg_typemap_mark(&this->typemap); for( i=0; infields; i++){ t_pg_coder *p_coder = this->convs[i].cconv; if( p_coder ) - rb_gc_mark(p_coder->coder_obj); + rb_gc_mark_movable(p_coder->coder_obj); + } +} + +static void +pg_tmbc_compact( t_tmbc *this ) +{ + int i; + + /* allocated but not initialized ? */ + if( this == (t_tmbc *)&pg_typemap_funcs ) return; + + pg_typemap_compact(&this->typemap); + for( i=0; infields; i++){ + t_pg_coder *p_coder = this->convs[i].cconv; + if( p_coder ) + pg_gc_location(p_coder->coder_obj); } } @@ -200,6 +216,7 @@ static const rb_data_type_t pg_tmbc_type = { (void (*)(void*))pg_tmbc_mark, (void (*)(void*))pg_tmbc_free, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_tmbc_compact), }, &pg_typemap_type, 0, diff --git a/ext/pg_type_map_by_mri_type.c b/ext/pg_type_map_by_mri_type.c index d1a1cf4dc..2e3be28f7 100644 --- a/ext/pg_type_map_by_mri_type.c +++ b/ext/pg_type_map_by_mri_type.c @@ -94,22 +94,34 @@ pg_tmbmt_fit_to_query( VALUE self, VALUE params ) } #define GC_MARK_AS_USED(type) \ - rb_gc_mark( this->coders.ask_##type ); \ - rb_gc_mark( this->coders.coder_obj_##type ); + rb_gc_mark_movable( this->coders.ask_##type ); \ + rb_gc_mark_movable( this->coders.coder_obj_##type ); static void pg_tmbmt_mark( t_tmbmt *this ) { - rb_gc_mark(this->typemap.default_typemap); + pg_typemap_mark(&this->typemap); FOR_EACH_MRI_TYPE( GC_MARK_AS_USED ); } +#define GC_COMPACT(type) \ + pg_gc_location( this->coders.ask_##type ); \ + pg_gc_location( this->coders.coder_obj_##type ); + +static void +pg_tmbmt_compact( t_tmbmt *this ) +{ + pg_typemap_compact(&this->typemap); + FOR_EACH_MRI_TYPE( GC_COMPACT ); +} + static const rb_data_type_t pg_tmbmt_type = { "PG::TypeMapByMriType", { (void (*)(void*))pg_tmbmt_mark, (void (*)(void*))-1, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_tmbmt_compact), }, &pg_typemap_type, 0, diff --git a/ext/pg_type_map_by_oid.c b/ext/pg_type_map_by_oid.c index e7985d4ed..b6a8be6df 100644 --- a/ext/pg_type_map_by_oid.c +++ b/ext/pg_type_map_by_oid.c @@ -158,9 +158,20 @@ pg_tmbo_mark( t_tmbo *this ) { int i; - rb_gc_mark(this->typemap.default_typemap); + pg_typemap_mark(&this->typemap); for( i=0; i<2; i++){ - rb_gc_mark(this->format[i].oid_to_coder); + rb_gc_mark_movable(this->format[i].oid_to_coder); + } +} + +static void +pg_tmbo_compact( t_tmbo *this ) +{ + int i; + + pg_typemap_compact(&this->typemap); + for( i=0; i<2; i++){ + pg_gc_location(this->format[i].oid_to_coder); } } @@ -170,6 +181,7 @@ static const rb_data_type_t pg_tmbo_type = { (void (*)(void*))pg_tmbo_mark, (void (*)(void*))-1, (size_t (*)(const void *))NULL, + pg_compact_callback(pg_tmbo_compact), }, &pg_typemap_type, 0, diff --git a/ext/pg_type_map_in_ruby.c b/ext/pg_type_map_in_ruby.c index 1bc99a07b..5ecddd421 100644 --- a/ext/pg_type_map_in_ruby.c +++ b/ext/pg_type_map_in_ruby.c @@ -6,20 +6,6 @@ #include "pg.h" -static const rb_data_type_t pg_tmir_type = { - "PG::TypeMapInRuby", - { - (void (*)(void*))NULL, - (void (*)(void*))-1, - (size_t (*)(const void *))NULL, - }, - &pg_typemap_type, - 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY - RUBY_TYPED_FREE_IMMEDIATELY, -#endif -}; - VALUE rb_cTypeMapInRuby; static VALUE s_id_fit_to_result; static VALUE s_id_fit_to_query; @@ -34,6 +20,28 @@ typedef struct { } t_tmir; +static void +pg_tmir_compact( t_tmir *this ) +{ + pg_typemap_compact(&this->typemap); + pg_gc_location(this->self); +} + +static const rb_data_type_t pg_tmir_type = { + "PG::TypeMapInRuby", + { + (void (*)(void*))pg_typemap_mark, + (void (*)(void*))-1, + (size_t (*)(const void *))NULL, + pg_compact_callback(pg_tmir_compact), + }, + &pg_typemap_type, + 0, +#ifdef RUBY_TYPED_FREE_IMMEDIATELY + RUBY_TYPED_FREE_IMMEDIATELY, +#endif +}; + /* * call-seq: * typemap.fit_to_result( result ) From c5dcadc29e685ea33e17dece584a1ef43bbb1d13 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Sun, 9 Aug 2020 17:56:54 +0200 Subject: [PATCH 6/9] Make all remaining T_DATA objects GC.compact friendly --- ext/pg_connection.c | 30 ++++++++++++++++++++++-------- ext/pg_result.c | 26 +++++++++++++++++++++----- ext/pg_tuple.c | 42 +++++++++++++++++++++++++++++++++--------- 3 files changed, 76 insertions(+), 22 deletions(-) diff --git a/ext/pg_connection.c b/ext/pg_connection.c index 86f3fbd55..2f56b5818 100644 --- a/ext/pg_connection.c +++ b/ext/pg_connection.c @@ -148,14 +148,27 @@ static const char *pg_cstr_enc(VALUE str, int enc_idx){ static void pgconn_gc_mark( t_pg_connection *this ) { - rb_gc_mark( this->socket_io ); - rb_gc_mark( this->notice_receiver ); - rb_gc_mark( this->notice_processor ); - rb_gc_mark( this->type_map_for_queries ); - rb_gc_mark( this->type_map_for_results ); - rb_gc_mark( this->trace_stream ); - rb_gc_mark( this->encoder_for_put_copy_data ); - rb_gc_mark( this->decoder_for_get_copy_data ); + rb_gc_mark_movable( this->socket_io ); + rb_gc_mark_movable( this->notice_receiver ); + rb_gc_mark_movable( this->notice_processor ); + rb_gc_mark_movable( this->type_map_for_queries ); + rb_gc_mark_movable( this->type_map_for_results ); + rb_gc_mark_movable( this->trace_stream ); + rb_gc_mark_movable( this->encoder_for_put_copy_data ); + rb_gc_mark_movable( this->decoder_for_get_copy_data ); +} + +static void +pgconn_gc_compact( t_pg_connection *this ) +{ + pg_gc_location( this->socket_io ); + pg_gc_location( this->notice_receiver ); + pg_gc_location( this->notice_processor ); + pg_gc_location( this->type_map_for_queries ); + pg_gc_location( this->type_map_for_results ); + pg_gc_location( this->trace_stream ); + pg_gc_location( this->encoder_for_put_copy_data ); + pg_gc_location( this->decoder_for_get_copy_data ); } @@ -181,6 +194,7 @@ static const rb_data_type_t pg_connection_type = { (void (*)(void*))pgconn_gc_mark, (void (*)(void*))pgconn_gc_free, (size_t (*)(const void *))NULL, + pg_compact_callback(pgconn_gc_compact), }, 0, 0, diff --git a/ext/pg_result.c b/ext/pg_result.c index 110ce95c6..7a1d3b2ab 100644 --- a/ext/pg_result.c +++ b/ext/pg_result.c @@ -111,13 +111,28 @@ pgresult_gc_mark( t_pg_result *this ) { int i; - rb_gc_mark( this->connection ); - rb_gc_mark( this->typemap ); - rb_gc_mark( this->tuple_hash ); - rb_gc_mark( this->field_map ); + rb_gc_mark_movable( this->connection ); + rb_gc_mark_movable( this->typemap ); + rb_gc_mark_movable( this->tuple_hash ); + rb_gc_mark_movable( this->field_map ); for( i=0; i < this->nfields; i++ ){ - rb_gc_mark( this->fnames[i] ); + rb_gc_mark_movable( this->fnames[i] ); + } +} + +static void +pgresult_gc_compact( t_pg_result *this ) +{ + int i; + + pg_gc_location( this->connection ); + pg_gc_location( this->typemap ); + pg_gc_location( this->tuple_hash ); + pg_gc_location( this->field_map ); + + for( i=0; i < this->nfields; i++ ){ + pg_gc_location( this->fnames[i] ); } } @@ -160,6 +175,7 @@ static const rb_data_type_t pgresult_type = { (void (*)(void*))pgresult_gc_mark, (void (*)(void*))pgresult_gc_free, (size_t (*)(const void *))pgresult_memsize, + pg_compact_callback(pgresult_gc_compact), }, 0, 0, #ifdef RUBY_TYPED_FREE_IMMEDIATELY diff --git a/ext/pg_tuple.c b/ext/pg_tuple.c index df4ba4ab9..39fcb9c19 100644 --- a/ext/pg_tuple.c +++ b/ext/pg_tuple.c @@ -52,30 +52,53 @@ typedef struct { VALUE values[0]; } t_pg_tuple; -static inline VALUE -pg_tuple_get_field_names( t_pg_tuple *this ) +static inline VALUE * +pg_tuple_get_field_names_ptr( t_pg_tuple *this ) { if( this->num_fields != (int)RHASH_SIZE(this->field_map) ){ - return this->values[this->num_fields]; + return &this->values[this->num_fields]; } else { - return Qfalse; + static VALUE f = Qfalse; + return &f; } } +static inline VALUE +pg_tuple_get_field_names( t_pg_tuple *this ) +{ + return *pg_tuple_get_field_names_ptr(this); +} + static void pg_tuple_gc_mark( t_pg_tuple *this ) { int i; if( !this ) return; - rb_gc_mark( this->result ); - rb_gc_mark( this->typemap ); - rb_gc_mark( this->field_map ); + rb_gc_mark_movable( this->result ); + rb_gc_mark_movable( this->typemap ); + rb_gc_mark_movable( this->field_map ); + + for( i = 0; i < this->num_fields; i++ ){ + rb_gc_mark_movable( this->values[i] ); + } + rb_gc_mark_movable( pg_tuple_get_field_names(this) ); +} + +static void +pg_tuple_gc_compact( t_pg_tuple *this ) +{ + int i; + + if( !this ) return; + pg_gc_location( this->result ); + pg_gc_location( this->typemap ); + pg_gc_location( this->field_map ); for( i = 0; i < this->num_fields; i++ ){ - rb_gc_mark( this->values[i] ); + pg_gc_location( this->values[i] ); } - rb_gc_mark( pg_tuple_get_field_names(this) ); + pg_gc_location( *pg_tuple_get_field_names_ptr(this) ); } static void @@ -98,6 +121,7 @@ static const rb_data_type_t pg_tuple_type = { (void (*)(void*))pg_tuple_gc_mark, (void (*)(void*))pg_tuple_gc_free, (size_t (*)(const void *))pg_tuple_memsize, + pg_compact_callback(pg_tuple_gc_compact), }, 0, 0, #ifdef RUBY_TYPED_FREE_IMMEDIATELY From a9c94189e5ae3cd65bcbe61e75567016d0adbadc Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Mon, 10 Aug 2020 17:21:44 +0200 Subject: [PATCH 7/9] Add truffleruby-head to travis-CI In the hope that the issue with TypedData inheritance is fixed there. --- .travis.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.travis.yml b/.travis.yml index e17e6679b..f4ec59e25 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,6 +31,10 @@ matrix: env: - "PGVERSION=12" + - rvm: truffleruby-head + env: + - "PGVERSION=12" + - name: gem-windows rvm: "2.7" before_install: @@ -43,6 +47,7 @@ matrix: - rake gem:windows:x86-mingw32 allow_failures: - rvm: ruby-head + - rvm: truffleruby-head fast_finish: true before_install: From 32a454996fa06ada22e2cf6606c1f184d6f8078e Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Fri, 14 Aug 2020 21:45:54 +0200 Subject: [PATCH 8/9] Use RUBY_TYPED_FREE_IMMEDIATELY unconditionally It is supported since ruby-2.1 and pg supports ruby-2.2+ --- ext/pg_coder.c | 6 ------ ext/pg_connection.c | 6 ------ ext/pg_copy_coder.c | 2 -- ext/pg_record_coder.c | 2 -- ext/pg_result.c | 2 -- ext/pg_tuple.c | 2 -- ext/pg_type_map.c | 2 -- ext/pg_type_map_all_strings.c | 2 -- ext/pg_type_map_by_class.c | 2 -- ext/pg_type_map_by_column.c | 2 -- ext/pg_type_map_by_mri_type.c | 2 -- ext/pg_type_map_by_oid.c | 2 -- ext/pg_type_map_in_ruby.c | 2 -- 13 files changed, 34 deletions(-) diff --git a/ext/pg_coder.c b/ext/pg_coder.c index d52fba673..53ea569a9 100644 --- a/ext/pg_coder.c +++ b/ext/pg_coder.c @@ -83,9 +83,7 @@ const rb_data_type_t pg_coder_type = { }, 0, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; static VALUE @@ -107,9 +105,7 @@ static const rb_data_type_t pg_composite_coder_type = { }, &pg_coder_type, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; static VALUE @@ -440,9 +436,7 @@ static const rb_data_type_t pg_coder_cfunc_type = { }, 0, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; void diff --git a/ext/pg_connection.c b/ext/pg_connection.c index 2f56b5818..3038956c1 100644 --- a/ext/pg_connection.c +++ b/ext/pg_connection.c @@ -198,9 +198,7 @@ static const rb_data_type_t pg_connection_type = { }, 0, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; @@ -1098,9 +1096,7 @@ static const rb_data_type_t pg_typecast_buffer_type = { }, 0, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; static char * @@ -1133,9 +1129,7 @@ static const rb_data_type_t pg_query_heap_pool_type = { }, 0, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; static int diff --git a/ext/pg_copy_coder.c b/ext/pg_copy_coder.c index 44593eb95..782c52dea 100644 --- a/ext/pg_copy_coder.c +++ b/ext/pg_copy_coder.c @@ -45,9 +45,7 @@ static const rb_data_type_t pg_copycoder_type = { }, &pg_coder_type, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; static VALUE diff --git a/ext/pg_record_coder.c b/ext/pg_record_coder.c index b6696a4df..bdf1e2c4a 100644 --- a/ext/pg_record_coder.c +++ b/ext/pg_record_coder.c @@ -38,9 +38,7 @@ static const rb_data_type_t pg_recordcoder_type = { }, &pg_coder_type, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; static VALUE diff --git a/ext/pg_result.c b/ext/pg_result.c index 7a1d3b2ab..5f7833546 100644 --- a/ext/pg_result.c +++ b/ext/pg_result.c @@ -178,9 +178,7 @@ static const rb_data_type_t pgresult_type = { pg_compact_callback(pgresult_gc_compact), }, 0, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; /* Needed by sequel_pg gem, do not delete */ diff --git a/ext/pg_tuple.c b/ext/pg_tuple.c index 39fcb9c19..d4e2f68b1 100644 --- a/ext/pg_tuple.c +++ b/ext/pg_tuple.c @@ -124,9 +124,7 @@ static const rb_data_type_t pg_tuple_type = { pg_compact_callback(pg_tuple_gc_compact), }, 0, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; /* diff --git a/ext/pg_type_map.c b/ext/pg_type_map.c index 757d25cbb..80a84ea29 100644 --- a/ext/pg_type_map.c +++ b/ext/pg_type_map.c @@ -28,9 +28,7 @@ const rb_data_type_t pg_typemap_type = { }, 0, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; VALUE rb_cTypeMap; diff --git a/ext/pg_type_map_all_strings.c b/ext/pg_type_map_all_strings.c index c1721ad4b..a010047b4 100644 --- a/ext/pg_type_map_all_strings.c +++ b/ext/pg_type_map_all_strings.c @@ -18,9 +18,7 @@ static const rb_data_type_t pg_tmas_type = { }, &pg_typemap_type, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; VALUE rb_cTypeMapAllStrings; diff --git a/ext/pg_type_map_by_class.c b/ext/pg_type_map_by_class.c index 498141cb9..4d9e3589e 100644 --- a/ext/pg_type_map_by_class.c +++ b/ext/pg_type_map_by_class.c @@ -148,9 +148,7 @@ static const rb_data_type_t pg_tmbk_type = { }, &pg_typemap_type, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; static VALUE diff --git a/ext/pg_type_map_by_column.c b/ext/pg_type_map_by_column.c index 3bbe1db7c..42e2a462a 100644 --- a/ext/pg_type_map_by_column.c +++ b/ext/pg_type_map_by_column.c @@ -220,9 +220,7 @@ static const rb_data_type_t pg_tmbc_type = { }, &pg_typemap_type, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; static VALUE diff --git a/ext/pg_type_map_by_mri_type.c b/ext/pg_type_map_by_mri_type.c index 2e3be28f7..a59f41fb4 100644 --- a/ext/pg_type_map_by_mri_type.c +++ b/ext/pg_type_map_by_mri_type.c @@ -125,9 +125,7 @@ static const rb_data_type_t pg_tmbmt_type = { }, &pg_typemap_type, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; #define INIT_VARIABLES(type) \ diff --git a/ext/pg_type_map_by_oid.c b/ext/pg_type_map_by_oid.c index b6a8be6df..56382bdc4 100644 --- a/ext/pg_type_map_by_oid.c +++ b/ext/pg_type_map_by_oid.c @@ -185,9 +185,7 @@ static const rb_data_type_t pg_tmbo_type = { }, &pg_typemap_type, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; static VALUE diff --git a/ext/pg_type_map_in_ruby.c b/ext/pg_type_map_in_ruby.c index 5ecddd421..3f29d7f08 100644 --- a/ext/pg_type_map_in_ruby.c +++ b/ext/pg_type_map_in_ruby.c @@ -37,9 +37,7 @@ static const rb_data_type_t pg_tmir_type = { }, &pg_typemap_type, 0, -#ifdef RUBY_TYPED_FREE_IMMEDIATELY RUBY_TYPED_FREE_IMMEDIATELY, -#endif }; /* From 3b3dc1dea4520c3a7c4e03cf914077d77ba1c5a4 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Mon, 10 Aug 2020 17:14:50 +0200 Subject: [PATCH 9/9] Add tests for compatibility with GC.compact @conninfo is not always defined in a describe context So define the constants in a before block and ensure the connection is closed. Moreover use rspec's :if filter to avoid indention "if: false" skips the execution of before, after and it blocks but the describe block is always executed. --- spec/pg/gc_compact_spec.rb | 103 +++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 spec/pg/gc_compact_spec.rb diff --git a/spec/pg/gc_compact_spec.rb b/spec/pg/gc_compact_spec.rb new file mode 100644 index 000000000..191b2e68c --- /dev/null +++ b/spec/pg/gc_compact_spec.rb @@ -0,0 +1,103 @@ +# -*- rspec -*- +# encoding: utf-8 +# +# Tests to verify correct implementation of compaction callbacks in rb_data_type_t definitions. +# +# Compaction callbacks update moved VALUEs. +# In ruby-2.7 they are invoked only while GC.compact or GC.verify_compaction_references. +# Ruby constants are usually moved, but local variables are not. +# +# Effectiveness of the tests below should be verified by commenting the compact callback out like so: +# +# static const rb_data_type_t pg_tmbo_type = { +# "PG::TypeMapByOid", +# { +# (void (*)(void*))pg_tmbo_mark, +# (void (*)(void*))-1, +# (size_t (*)(const void *))NULL, +# // pg_compact_callback(pg_tmbo_compact), +# }, +# +# This should result in a segmentation fault aborting the whole process. +# Therefore the effectiveness of only one test can be verified per rspec run. + +require_relative '../helpers' + +describe "GC.compact", if: GC.respond_to?(:compact) do + before :all do + TM1 = Class.new(PG::TypeMapByClass) do + def conv_array(value) + PG::TextEncoder::JSON.new + end + end.new + TM1[Array] = :conv_array + + E1 = PG::TextEncoder::JSON.new + + TM2 = PG::TypeMapByClass.new + TM2.default_type_map = PG::TypeMapInRuby.new + + TMBC = PG::TypeMapByColumn.new([PG::TextDecoder::Float.new]) + + + CONN2 = PG.connect(@conninfo) + CONN2.type_map_for_results = PG::BasicTypeMapForResults.new(CONN2) + + RES1 = CONN2.exec("SELECT 234") + + TUP1 = RES1.tuple(0) + + TM3 = PG::TypeMapByClass.new + CPYENC = PG::TextEncoder::CopyRow.new type_map: TM3 + RECENC = PG::TextEncoder::Record.new type_map: TM3 + + # Use GC.verify_compaction_references instead of GC.compact . + # This has the advantage that all movable objects are actually moved. + # The downside is that it doubles the heap space of the Ruby process. + # Therefore we call it only once and do several tests afterwards. + GC.verify_compaction_references(toward: :empty, double_heap: true) + end + + it "should compact PG::TypeMapByClass #328" do + res = @conn.exec_params("SELECT $1", [[5]], 0, TM1) + expect( res.getvalue(0, 0) ).to eq( "[5]" ) + end + + it "should compact PG::CompositeCoder #327" do + e2 = PG::TextEncoder::Array.new elements_type: E1 + expect( e2.encode([5]) ).to eq("{5}") + end + + it "should compact PG::TypeMap#default_type_map" do + expect( TM2.default_type_map ).to be_kind_of( PG::TypeMapInRuby ) + end + + it "should compact PG::TypeMapByColumn" do + res = CONN2.exec("SELECT 555") + expect( res.getvalue(0,0) ).to eq( 555 ) + end + + it "should compact PG::Connection" do + expect( TMBC.coders[0] ).to be_kind_of(PG::TextDecoder::Float) + end + + it "should compact PG::Result" do + expect( RES1.getvalue(0,0) ).to eq( 234 ) + end + + it "should compact PG::Tuple" do + expect( TUP1[0] ).to eq( 234 ) + end + + it "should compact PG::CopyCoder" do + expect( CPYENC.encode([45]) ).to eq( "45\n" ) + end + + it "should compact PG::RecordCoder" do + expect( RECENC.encode([34]) ).to eq( '("34")' ) + end + + after :all do + CONN2.close + end +end