You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@thrift.apache.org by adam maxiaodong <ad...@gmail.com> on 2013/03/26 22:22:46 UTC
Reporting a crash issue in t_c_glib_generator.cc
Posting here as suggested by Roger.
Hello,
A crash is introduced by THRIFT-1583 c_glib leaks memory
https://git-wip-us.apache.org/**repos/asf?p=thrift.git;a=**commit;h=**
c75797d9060e049692c5db1617aa95**60aec939c8<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=commit;h=c75797d9060e049692c5db1617aa9560aec939c8>
in t_c_**glib_generator.cc line 1860 and 2529, g_free() is incorrectly
used for type string.
@@ -1826,10<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=d22b72324fff1134fcfeea90d5f93be1a49e7cfa;hb=d22b72324fff1134fcfeea90d5f93be1a49e7cfa#l1826>
+1828,39<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=b088299c8f8d8de20b35742f679989178b1e93a6;hb=b088299c8f8d8de20b35742f679989178b1e93a6#l1828>@@
void t_c_glib_generator::generate_object(t_struct *tstruct) {
}
}
+ f_types_impl_ << indent() << "if (tobject->" << name << " !=
NULL)" << endl;
+ f_types_impl_ << indent() << "{" << endl;
+ indent_up();
f_types_impl_ <<
indent() << destructor_function << " (tobject->" << name <<
- ", FALSE);" << endl;
+ ", TRUE);" << endl;
+ f_types_impl_ << indent() << "tobject->" << name << " = NULL;" << endl;
+ indent_down();
+ f_types_impl_ << indent() << "}" << endl;
}
+ } else if (t->is_struct() || t->is_xception()) {
+ string name = (*m_iter)->get_name();
+ // TODO: g_clear_object needs glib >= 2.28
+ // f_types_impl_ << indent() << "g_clear_object (&(tobject->"
<< name << "));" << endl;
+ // does g_object_unref the trick?
+ f_types_impl_ << indent() << "if (tobject->" << name << " !=
NULL)" << endl;
+ f_types_impl_ << indent() << "{" << endl;
+ indent_up();
+ f_types_impl_ <<
+ indent() << "g_object_unref(tobject->" << name << ");" << endl;
+ f_types_impl_ << indent() << "tobject->" << name << " = NULL;" << endl;
+ indent_down();
+ f_types_impl_ << indent() << "}" << endl;
+ } else if (t->is_string()) {
+ string name = (*m_iter)->get_name();
+ f_types_impl_ << indent() << "if (tobject->" << name << " !=
NULL)" << endl;
+ f_types_impl_ << indent() << "{" << endl;
+ indent_up();
+ f_types_impl_ <<
+ indent() << "g_free (tobject->" << name << ");" << endl;
//////////////////////////////////////////////// here
+ f_types_impl_ << indent() << "tobject->" << name << " = NULL;" << endl;
+ indent_down();
+ f_types_impl_ << indent() << "}" << endl;
}
}
and
@@ -2451,12<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=d22b72324fff1134fcfeea90d5f93be1a49e7cfa;hb=d22b72324fff1134fcfeea90d5f93be1a49e7cfa#l2451>
+2517,21<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=b088299c8f8d8de20b35742f679989178b1e93a6;hb=b088299c8f8d8de20b35742f679989178b1e93a6#l2517>@@
void t_c_glib_generator::generate_deserialize_field(ofstream &out,
string name = prefix + tfield->get_name() + suffix;
if (type->is_struct() || type->is_xception()) {
- generate_deserialize_struct (out, (t_struct *) type, name, error_ret);
+ generate_deserialize_struct (out, (t_struct *) type, name,
error_ret, allocate);
} else if (type->is_container()) {
generate_deserialize_container (out, type, name, error_ret);
} else if (type->is_base_type()) {
t_base_type::t_base tbase = ((t_base_type *) type)->get_base();
-
+ if (tbase == t_base_type::TYPE_STRING) {
+ indent(out) << "if (" << name << " != NULL)" << endl <<
+ indent() << "{" << endl;
+ indent_up();
+ indent(out) << "g_free(" << name << ");" << endl <<
//////////////////////////////////////////////// and here!
+ indent() << name << " = NULL;" << endl;
+ indent_down();
+ indent(out) << "}" << endl <<
+ endl;
+ }
indent(out) << "if ((ret = thrift_protocol_read_";
I am wondering if anyone also hit this issue, please suggest a fix.
Thanks!
-Adam
Re: Reporting a crash issue in t_c_glib_generator.cc
Posted by adam maxiaodong <ad...@gmail.com>.
Roger,
I don't know how to add a patch, basically here is the test case:
thrift:
binary mybinary,
and here is the fix:
use g_byte_array_free() instead of free.
I am wondering if you can help to add this test case and commit the fix.
thanks!
-Adam
On Thu, Mar 28, 2013 at 3:11 PM, Roger Meier <ro...@bufferoverflow.ch>wrote:
> Could you please add a patch containing a test case and the fix here:
> https://issues.apache.org/**jira/browse/THRIFT-1583<https://issues.apache.org/jira/browse/THRIFT-1583>
>
> thanks!
> ;-r
>
> Quoting adam maxiaodong <ad...@gmail.com>:
>
> Posting here as suggested by Roger.
>>
>> Hello,
>>
>> A crash is introduced by THRIFT-1583 c_glib leaks memory
>> https://git-wip-us.apache.org/****repos/asf?p=thrift.git;a=****
>> commit;h=**<https://git-wip-us.apache.org/**repos/asf?p=thrift.git;a=**commit;h=**>
>> c75797d9060e049692c5db1617aa95****60aec939c8<https://git-wip-**
>> us.apache.org/repos/asf?p=**thrift.git;a=commit;h=**
>> c75797d9060e049692c5db1617aa95**60aec939c8<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=commit;h=c75797d9060e049692c5db1617aa9560aec939c8>
>> >
>> in t_c_**glib_generator.cc line 1860 and 2529, g_free() is incorrectly
>> used for type string.
>>
>> @@ -1826,10<https://git-wip-us.**apache.org/repos/asf?p=thrift.**
>> git;a=blob;f=compiler/cpp/src/**generate/t_c_glib_generator.**cc;h=**
>> d22b72324fff1134fcfeea90d5f93b**e1a49e7cfa;hb=**
>> d22b72324fff1134fcfeea90d5f93b**e1a49e7cfa#l1826<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=d22b72324fff1134fcfeea90d5f93be1a49e7cfa;hb=d22b72324fff1134fcfeea90d5f93be1a49e7cfa#l1826>
>> >
>> +1828,39<https://git-wip-us.**apache.org/repos/asf?p=thrift.**
>> git;a=blob;f=compiler/cpp/src/**generate/t_c_glib_generator.**cc;h=**
>> b088299c8f8d8de20b35742f679989**178b1e93a6;hb=**
>> b088299c8f8d8de20b35742f679989**178b1e93a6#l1828<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=b088299c8f8d8de20b35742f679989178b1e93a6;hb=b088299c8f8d8de20b35742f679989178b1e93a6#l1828>>@@
>>
>>
>> void t_c_glib_generator::generate_**object(t_struct *tstruct) {
>> }
>> }
>>
>> + f_types_impl_ << indent() << "if (tobject->" << name << " !=
>> NULL)" << endl;
>> + f_types_impl_ << indent() << "{" << endl;
>> + indent_up();
>> f_types_impl_ <<
>> indent() << destructor_function << " (tobject->" << name <<
>> - ", FALSE);" << endl;
>> + ", TRUE);" << endl;
>> + f_types_impl_ << indent() << "tobject->" << name << " = NULL;"
>> << endl;
>> + indent_down();
>> + f_types_impl_ << indent() << "}" << endl;
>> }
>> + } else if (t->is_struct() || t->is_xception()) {
>> + string name = (*m_iter)->get_name();
>> + // TODO: g_clear_object needs glib >= 2.28
>> + // f_types_impl_ << indent() << "g_clear_object (&(tobject->"
>> << name << "));" << endl;
>> + // does g_object_unref the trick?
>> + f_types_impl_ << indent() << "if (tobject->" << name << " !=
>> NULL)" << endl;
>> + f_types_impl_ << indent() << "{" << endl;
>> + indent_up();
>> + f_types_impl_ <<
>> + indent() << "g_object_unref(tobject->" << name << ");" << endl;
>> + f_types_impl_ << indent() << "tobject->" << name << " = NULL;" <<
>> endl;
>> + indent_down();
>> + f_types_impl_ << indent() << "}" << endl;
>> + } else if (t->is_string()) {
>> + string name = (*m_iter)->get_name();
>> + f_types_impl_ << indent() << "if (tobject->" << name << " !=
>> NULL)" << endl;
>> + f_types_impl_ << indent() << "{" << endl;
>> + indent_up();
>> + f_types_impl_ <<
>> + indent() << "g_free (tobject->" << name << ");" << endl;
>> //////////////////////////////**////////////////// here
>> + f_types_impl_ << indent() << "tobject->" << name << " = NULL;" <<
>> endl;
>> + indent_down();
>> + f_types_impl_ << indent() << "}" << endl;
>> }
>> }
>>
>> and
>>
>>
>> @@ -2451,12<https://git-wip-us.**apache.org/repos/asf?p=thrift.**
>> git;a=blob;f=compiler/cpp/src/**generate/t_c_glib_generator.**cc;h=**
>> d22b72324fff1134fcfeea90d5f93b**e1a49e7cfa;hb=**
>> d22b72324fff1134fcfeea90d5f93b**e1a49e7cfa#l2451<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=d22b72324fff1134fcfeea90d5f93be1a49e7cfa;hb=d22b72324fff1134fcfeea90d5f93be1a49e7cfa#l2451>
>> >
>> +2517,21<https://git-wip-us.**apache.org/repos/asf?p=thrift.**
>> git;a=blob;f=compiler/cpp/src/**generate/t_c_glib_generator.**cc;h=**
>> b088299c8f8d8de20b35742f679989**178b1e93a6;hb=**
>> b088299c8f8d8de20b35742f679989**178b1e93a6#l2517<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=b088299c8f8d8de20b35742f679989178b1e93a6;hb=b088299c8f8d8de20b35742f679989178b1e93a6#l2517>>@@
>>
>>
>> void t_c_glib_generator::generate_**deserialize_field(ofstream &out,
>> string name = prefix + tfield->get_name() + suffix;
>>
>> if (type->is_struct() || type->is_xception()) {
>> - generate_deserialize_struct (out, (t_struct *) type, name,
>> error_ret);
>> + generate_deserialize_struct (out, (t_struct *) type, name,
>> error_ret, allocate);
>> } else if (type->is_container()) {
>> generate_deserialize_container (out, type, name, error_ret);
>> } else if (type->is_base_type()) {
>> t_base_type::t_base tbase = ((t_base_type *) type)->get_base();
>> -
>> + if (tbase == t_base_type::TYPE_STRING) {
>> + indent(out) << "if (" << name << " != NULL)" << endl <<
>> + indent() << "{" << endl;
>> + indent_up();
>> + indent(out) << "g_free(" << name << ");" << endl <<
>> //////////////////////////////**////////////////// and here!
>> + indent() << name << " = NULL;" << endl;
>> + indent_down();
>> + indent(out) << "}" << endl <<
>> + endl;
>> + }
>> indent(out) << "if ((ret = thrift_protocol_read_";
>>
>>
>> I am wondering if anyone also hit this issue, please suggest a fix.
>>
>> Thanks!
>> -Adam
>>
>
>
>
Re: Reporting a crash issue in t_c_glib_generator.cc
Posted by Roger Meier <ro...@bufferoverflow.ch>.
Could you please add a patch containing a test case and the fix here:
https://issues.apache.org/jira/browse/THRIFT-1583
thanks!
;-r
Quoting adam maxiaodong <ad...@gmail.com>:
> Posting here as suggested by Roger.
>
> Hello,
>
> A crash is introduced by THRIFT-1583 c_glib leaks memory
> https://git-wip-us.apache.org/**repos/asf?p=thrift.git;a=**commit;h=**
> c75797d9060e049692c5db1617aa95**60aec939c8<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=commit;h=c75797d9060e049692c5db1617aa9560aec939c8>
> in t_c_**glib_generator.cc line 1860 and 2529, g_free() is incorrectly
> used for type string.
>
> @@
> -1826,10<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=d22b72324fff1134fcfeea90d5f93be1a49e7cfa;hb=d22b72324fff1134fcfeea90d5f93be1a49e7cfa#l1826>
> +1828,39<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=b088299c8f8d8de20b35742f679989178b1e93a6;hb=b088299c8f8d8de20b35742f679989178b1e93a6#l1828>@@
> void t_c_glib_generator::generate_object(t_struct *tstruct) {
> }
> }
>
> + f_types_impl_ << indent() << "if (tobject->" << name << " !=
> NULL)" << endl;
> + f_types_impl_ << indent() << "{" << endl;
> + indent_up();
> f_types_impl_ <<
> indent() << destructor_function << " (tobject->" << name <<
> - ", FALSE);" << endl;
> + ", TRUE);" << endl;
> + f_types_impl_ << indent() << "tobject->" << name << " =
> NULL;" << endl;
> + indent_down();
> + f_types_impl_ << indent() << "}" << endl;
> }
> + } else if (t->is_struct() || t->is_xception()) {
> + string name = (*m_iter)->get_name();
> + // TODO: g_clear_object needs glib >= 2.28
> + // f_types_impl_ << indent() << "g_clear_object (&(tobject->"
> << name << "));" << endl;
> + // does g_object_unref the trick?
> + f_types_impl_ << indent() << "if (tobject->" << name << " !=
> NULL)" << endl;
> + f_types_impl_ << indent() << "{" << endl;
> + indent_up();
> + f_types_impl_ <<
> + indent() << "g_object_unref(tobject->" << name << ");" << endl;
> + f_types_impl_ << indent() << "tobject->" << name << " =
> NULL;" << endl;
> + indent_down();
> + f_types_impl_ << indent() << "}" << endl;
> + } else if (t->is_string()) {
> + string name = (*m_iter)->get_name();
> + f_types_impl_ << indent() << "if (tobject->" << name << " !=
> NULL)" << endl;
> + f_types_impl_ << indent() << "{" << endl;
> + indent_up();
> + f_types_impl_ <<
> + indent() << "g_free (tobject->" << name << ");" << endl;
> //////////////////////////////////////////////// here
> + f_types_impl_ << indent() << "tobject->" << name << " =
> NULL;" << endl;
> + indent_down();
> + f_types_impl_ << indent() << "}" << endl;
> }
> }
>
> and
>
>
> @@
> -2451,12<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=d22b72324fff1134fcfeea90d5f93be1a49e7cfa;hb=d22b72324fff1134fcfeea90d5f93be1a49e7cfa#l2451>
> +2517,21<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=b088299c8f8d8de20b35742f679989178b1e93a6;hb=b088299c8f8d8de20b35742f679989178b1e93a6#l2517>@@
> void t_c_glib_generator::generate_deserialize_field(ofstream &out,
> string name = prefix + tfield->get_name() + suffix;
>
> if (type->is_struct() || type->is_xception()) {
> - generate_deserialize_struct (out, (t_struct *) type, name, error_ret);
> + generate_deserialize_struct (out, (t_struct *) type, name,
> error_ret, allocate);
> } else if (type->is_container()) {
> generate_deserialize_container (out, type, name, error_ret);
> } else if (type->is_base_type()) {
> t_base_type::t_base tbase = ((t_base_type *) type)->get_base();
> -
> + if (tbase == t_base_type::TYPE_STRING) {
> + indent(out) << "if (" << name << " != NULL)" << endl <<
> + indent() << "{" << endl;
> + indent_up();
> + indent(out) << "g_free(" << name << ");" << endl <<
> //////////////////////////////////////////////// and here!
> + indent() << name << " = NULL;" << endl;
> + indent_down();
> + indent(out) << "}" << endl <<
> + endl;
> + }
> indent(out) << "if ((ret = thrift_protocol_read_";
>
>
> I am wondering if anyone also hit this issue, please suggest a fix.
>
> Thanks!
> -Adam