Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-2182

segfault in regression tests (GC bug in rb_thrift_memory_buffer_write)

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.1, 0.9.2
    • Fix Version/s: 0.9.2
    • Component/s: Ruby - Library
    • Labels:
    • Patch Info:
      Patch Available

      Description

      This bug causes the regression tests to segfault on my machine. As this is a GC bug, it may or may not be easily reproducible.

      The rb_thrift_memory_buffer_write function looks like this:

          VALUE rb_thrift_memory_buffer_write(VALUE self, VALUE str) {
            VALUE buf = GET_BUF(self);
            str = force_binary_encoding(str);
            rb_str_buf_cat(buf, RSTRING_PTR(str), RSTRING_LEN(str));
            return Qnil;
          }
      

      When gcc compiles this, it optimizes away the value of str (it is no longer used after RSTRING_PTR(str) and RSTRING_LEN(str) are computed). Later, rb_str_buf_cat invokes the GC, and the string referenced by str is collected, because there are no references to it on the stack.

      Some possible solutions:

      • Use StringValuePtr instead of RSTRING_PTR (in general RSTRING_PTR should be avoided in favor of StringValuePtr or StringValueCStr); I believe this will also fix #THRIFT-1047
      • Use rb_str_cat instead of rb_str_buf_cat
      • Use RB_GC_GUARD to prevent str from getting collected

      It appears a similar bug may exist with buffer_value in rb_thrift_memory_buffer_read_into_buffer, and possibly in any of the other 30 places that RSTRING_PTR is used.

        Attachments

        Issue Links

          Activity

            People

            • Assignee:
              roger Roger Meier
              Reporter:
              cout Paul Brannan

              Dates

              • Created:
                Updated:
                Resolved:

                Issue deployment