[
https://issues.apache.org/jira/browse/THRIFT-1389?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14139006#comment-14139006
]
Simon South commented on THRIFT-1389:
-------------------------------------
I believe this issue can be closed. It was apparently fixed as part of
[THRIFT-1583, "c_glib leaks
memory"|https://issues.apache.org/jira/browse/THRIFT-1583] (in [this
commit|https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=commit;h=c75797d9060e049692c5db1617aa9560aec939c8]).
{{t_c_glib_generator::generate_deserialize_struct}} was modified so the
generated code no longer creates a new object (a {{Column}}, in this case) but
rather invokes {{thrift_struct_read}} on the one already created by the
containing object's constructor.
Here's the relevant portion of the code generated for
{{column_or_super_column_read}} by the compiler today:
{noformat}
switch (fid)
{
case 1:
if (ftype == T_STRUCT)
{
if ((ret = thrift_struct_read (THRIFT_STRUCT (this_object->column),
protocol, error)) < 0)
...
{noformat}
No superfluous call to {{g_object_new}} and hence no memory leak.
I'll respond to Andris' comment above as it may be relevant to future
contributors:
The change proposed by Jerry addresses the memory leak in this case, but does
so by modifying the containing object's constructor so it does not initialize
its member {{Column}} object during construction. This violates a principle
observed throughout the rest of the C implementation, which is that
*serializable objects are initialized as early as possible*.
This is done (I believe) because Thrift itself has no notion of a null object.
If, for instance, a service method is defined as returning a map but for a
particular invocation the method has no data to return, what the client
receives is _an empty map_, not a null pointer or similar. As a result there is
no reason for a serializable object or its members to ever _not_ be
initialized, as there is no way of representing them within the Thrift protocol
otherwise. (This is my understanding, at least—perhaps someone more
knowledgeable will correct me.)
So with Jerry's fix applied, newly constructed {{ColumnOrSuperColumn}} objects
cannot be serialized at all. The correct fix was to stop the generated code
from blindly re-initializing serializable objects' members; the compiler should
know they are always initialized at construction.
> c_glib_generator.cc generates leaking code for cassandra_client_get_slice()
> and cassandra_client_get()
> ------------------------------------------------------------------------------------------------------
>
> Key: THRIFT-1389
> URL: https://issues.apache.org/jira/browse/THRIFT-1389
> Project: Thrift
> Issue Type: Bug
> Components: C glib - Compiler, C glib - Library
> Affects Versions: 0.6.1
> Environment: Used with Cassandra 0.8.4 on Linux 2.6.32 with x86_64
> Reporter: Jerry Gally
> Assignee: Simon South
>
> The generated cassandra.c {{cassandra_client_recv_get_slice()}} code for list
> processing instantiates an object of {{TYPE_COLUMN_OR_SUPER_COLUMN}} then
> calls {{thrift_struct_read()}} as in the following code snippet:
> {noformat}
> switch (fid)
> {
> case 0:
> if (ftype == T_LIST)
> {
> {
> guint32 size;
> ThriftType element_type;
> if ((ret = thrift_protocol_read_list_begin (protocol,
> &element_type, &size, error)) < 0)
> return 0;
> xfer += ret;
> /* iterate through list elements */
> guint32 i;
> for (i = 0; i < size; i++)
> {
> ColumnOrSuperColumn * _elem19;
> _elem19 = g_object_new (TYPE_COLUMN_OR_SUPER_COLUMN, NULL);
> //printf("\nrecv_slice made %p\n", _elem19->column);
> if ((ret = thrift_struct_read (THRIFT_STRUCT (_elem19),
> protocol, error)) < 0)
> {noformat}
> When the object of {{TYPE_COLUMN_OR_SUPER_COLUMN}} is instantiated, it in
> turn instantiates a child of {{TYPE_COLUMN}}:
> {noformat}
> void
> column_or_super_column_instance_init (ColumnOrSuperColumn * object)
> {
> /* satisfy -Wall */
> THRIFT_UNUSED_VAR (object);
> object->column = g_object_new (TYPE_COLUMN, NULL);
> object->__isset_column = FALSE;
> {noformat}
> But this instance reference is lost and replaced by a new {{TYPE_COLUMN}}
> instantiation reference when the column member is read by
> {{column_or_super_column_read()}} within the same execution context of the
> top level {{cassandra_client_recv_get_slice()}} call:
> {noformat}
> switch (fid)
> {
> case 1:
> if (ftype == T_STRUCT)
> {
> this_object->column = g_object_new (TYPE_COLUMN, NULL);
> if ((ret = thrift_struct_read (THRIFT_STRUCT (this_object->column),
> protocol, error)) < 0)
> {noformat}
> The above snippets/logic/leak described above for
> {{cassandra_client_get_slice()}} also apply for {{cassandra_client_get()}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
