Thrift
  1. Thrift
  2. THRIFT-1389

c_glib_generator.cc generates leaking code for cassandra_client_get_slice() and cassandra_client_get()

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 0.6.1
    • Fix Version/s: None
    • Labels:
      None
    • Environment:

      Used with Cassandra 0.8.4 on Linux 2.6.32 with x86_64

      Description

      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,:

      <... snip>
      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)

      <\snip ...>

      When the object of TYPE_COLUMN_OR_SUPER_COLUMN is instantiated, it in-turn instantiates a child of TYPE_COLUMN:

      <... snip>
      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;

      </snip ...>

      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:

      <... snip>

      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)

      </snip ...>

      The above snippits/logic/leak described above for cassandra_client_get_slice() also apply for cassandra_client_get().

        Activity

        Hide
        Jerry Gally added a comment -

        When I ran accross this issue last year, and went searching for kindred spirits, I was left with the distinct impression that the C Thrift community was, as far as I could tell, virtually non-existent beyond the original _c_glib_generator.cc developer/contributer.

        Recieving your follow-up question on this issue is kind of like detecting a radio signal in the Search for Extra-Terrestrial Intellgence (SETI) program

        I, on the other hand, am no longer delving into the C Thrift context, but I do hope you find kindred spirits 8-)

        Show
        Jerry Gally added a comment - When I ran accross this issue last year, and went searching for kindred spirits, I was left with the distinct impression that the C Thrift community was, as far as I could tell, virtually non-existent beyond the original _c_glib_generator.cc developer/contributer. Recieving your follow-up question on this issue is kind of like detecting a radio signal in the Search for Extra-Terrestrial Intellgence (SETI) program I, on the other hand, am no longer delving into the C Thrift context, but I do hope you find kindred spirits 8-)
        Hide
        Andris Mednis added a comment -

        The fix in t_c_glib_generator.cc proposed by Jerry Gally helped to fix memory leak when Thrift 0.7.0 was used to generate files for connecting to Cassandra 1.0.7 from a C application. Does anybody know why the fix is not included in Thrift?

        Show
        Andris Mednis added a comment - The fix in t_c_glib_generator.cc proposed by Jerry Gally helped to fix memory leak when Thrift 0.7.0 was used to generate files for connecting to Cassandra 1.0.7 from a C application. Does anybody know why the fix is not included in Thrift?
        Hide
        Jerry Gally added a comment -

        The following change to t_c_glib_generator.cc appears to fix the leak:

        1733c1733
        < indent(f_types_impl_) << "object->" << name << " = g_object_new (" << this->nspace_uc << "TYPE_" << type_name_uc << ", NULL);" << endl;

        > indent(f_types_impl_) << "object->" << name << " = NULL;" << endl;

        Show
        Jerry Gally added a comment - The following change to t_c_glib_generator.cc appears to fix the leak: 1733c1733 < indent(f_types_impl_) << "object->" << name << " = g_object_new (" << this->nspace_uc << "TYPE_" << type_name_uc << ", NULL);" << endl; — > indent(f_types_impl_) << "object->" << name << " = NULL;" << endl;
        Hide
        Jerry Gally added a comment -

        The following valgrind leak reports initially exposed the leaks, and manually placed printf's in the generated cassandra code demonstrated column instantiation followed by re-instantiation and reference lost upon read:

        ==5051== 9,072 bytes in 126 blocks are definitely lost in loss record 527 of 528
        ==5051== at 0x4C2110C: malloc (vg_replace_malloc.c:195)
        ==5051== by 0x52ABBFA: g_malloc (in /lib64/libglib-2.0.so.0.1200.3)
        ==5051== by 0x52BB305: g_slice_alloc0 (in /lib64/libglib-2.0.so.0.1200.3)
        ==5051== by 0x50617CE: g_type_create_instance (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x50498BC: ??? (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x5047723: g_object_newv (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x504814B: g_object_new_valist (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x5048380: g_object_new (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x41E6BB: column_or_super_column_instance_init (cassandra_types.c:1148)
        ==5051== by 0x5061C8F: g_type_create_instance (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x50498BC: ??? (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x5047723: g_object_newv (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x504814B: g_object_new_valist (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x5048380: g_object_new (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x409E2C: cassandra_client_recv_get (cassandra.c:792)
        ==5051== by 0x40A3A6: cassandra_client_get (cassandra.c:904)
        ==5051== by 0x40723E: CregListNodeRecursively (sampleclient.c:952)
        ==5051== by 0x4070EB: CregListNodeRecursively (sampleclient.c:931)
        ==5051== by 0x4070EB: CregListNodeRecursively (sampleclient.c:931)
        ==5051== by 0x406D8E: CregListNodes (sampleclient.c:842)
        ==5051== by 0x4078FD: main (sampleclient.c:1133)

        ==5051== 21,168 bytes in 294 blocks are definitely lost in loss record 528 of 528
        ==5051== at 0x4C2110C: malloc (vg_replace_malloc.c:195)
        ==5051== by 0x52ABBFA: g_malloc (in /lib64/libglib-2.0.so.0.1200.3)
        ==5051== by 0x52BB305: g_slice_alloc0 (in /lib64/libglib-2.0.so.0.1200.3)
        ==5051== by 0x50617CE: g_type_create_instance (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x50498BC: ??? (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x5047723: g_object_newv (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x504814B: g_object_new_valist (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x5048380: g_object_new (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x41E6BB: column_or_super_column_instance_init (cassandra_types.c:1148)
        ==5051== by 0x5061C8F: g_type_create_instance (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x50498BC: ??? (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x5047723: g_object_newv (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x504814B: g_object_new_valist (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x5048380: g_object_new (in /lib64/libgobject-2.0.so.0.1200.3)
        ==5051== by 0x40AB90: cassandra_client_recv_get_slice (cassandra.c:1078)
        ==5051== by 0x40B0A1: cassandra_client_get_slice (cassandra.c:1179)
        ==5051== by 0x406F58: CregListNodeRecursively (sampleclient.c:893)
        ==5051== by 0x4070EB: CregListNodeRecursively (sampleclient.c:931)
        ==5051== by 0x4070EB: CregListNodeRecursively (sampleclient.c:931)
        ==5051== by 0x406D8E: CregListNodes (sampleclient.c:842)
        ==5051== by 0x4078FD: main (sampleclient.c:1133)

        Show
        Jerry Gally added a comment - The following valgrind leak reports initially exposed the leaks, and manually placed printf's in the generated cassandra code demonstrated column instantiation followed by re-instantiation and reference lost upon read: ==5051== 9,072 bytes in 126 blocks are definitely lost in loss record 527 of 528 ==5051== at 0x4C2110C: malloc (vg_replace_malloc.c:195) ==5051== by 0x52ABBFA: g_malloc (in /lib64/libglib-2.0.so.0.1200.3) ==5051== by 0x52BB305: g_slice_alloc0 (in /lib64/libglib-2.0.so.0.1200.3) ==5051== by 0x50617CE: g_type_create_instance (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x50498BC: ??? (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x5047723: g_object_newv (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x504814B: g_object_new_valist (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x5048380: g_object_new (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x41E6BB: column_or_super_column_instance_init (cassandra_types.c:1148) ==5051== by 0x5061C8F: g_type_create_instance (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x50498BC: ??? (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x5047723: g_object_newv (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x504814B: g_object_new_valist (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x5048380: g_object_new (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x409E2C: cassandra_client_recv_get (cassandra.c:792) ==5051== by 0x40A3A6: cassandra_client_get (cassandra.c:904) ==5051== by 0x40723E: CregListNodeRecursively (sampleclient.c:952) ==5051== by 0x4070EB: CregListNodeRecursively (sampleclient.c:931) ==5051== by 0x4070EB: CregListNodeRecursively (sampleclient.c:931) ==5051== by 0x406D8E: CregListNodes (sampleclient.c:842) ==5051== by 0x4078FD: main (sampleclient.c:1133) ==5051== 21,168 bytes in 294 blocks are definitely lost in loss record 528 of 528 ==5051== at 0x4C2110C: malloc (vg_replace_malloc.c:195) ==5051== by 0x52ABBFA: g_malloc (in /lib64/libglib-2.0.so.0.1200.3) ==5051== by 0x52BB305: g_slice_alloc0 (in /lib64/libglib-2.0.so.0.1200.3) ==5051== by 0x50617CE: g_type_create_instance (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x50498BC: ??? (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x5047723: g_object_newv (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x504814B: g_object_new_valist (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x5048380: g_object_new (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x41E6BB: column_or_super_column_instance_init (cassandra_types.c:1148) ==5051== by 0x5061C8F: g_type_create_instance (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x50498BC: ??? (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x5047723: g_object_newv (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x504814B: g_object_new_valist (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x5048380: g_object_new (in /lib64/libgobject-2.0.so.0.1200.3) ==5051== by 0x40AB90: cassandra_client_recv_get_slice (cassandra.c:1078) ==5051== by 0x40B0A1: cassandra_client_get_slice (cassandra.c:1179) ==5051== by 0x406F58: CregListNodeRecursively (sampleclient.c:893) ==5051== by 0x4070EB: CregListNodeRecursively (sampleclient.c:931) ==5051== by 0x4070EB: CregListNodeRecursively (sampleclient.c:931) ==5051== by 0x406D8E: CregListNodes (sampleclient.c:842) ==5051== by 0x4078FD: main (sampleclient.c:1133)

          People

          • Assignee:
            Unassigned
            Reporter:
            Jerry Gally
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development