memory leak if to_json method on class raises an error (C ext)

[luke-gru]

Hi,

I'm not sure if this is a bug, or just undocumented behaviour, but here's a script to reproduce the memory leak:

require 'json'

class MyClass
  def to_json(*)
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" * 200000
  end
end

class MyOther
  def to_json(*)
    raise "OMG"
  end
end

1000.times do |i|
  puts i
  JSON.dump([MyClass.new, MyClass.new, MyClass.new, MyOther.new]) rescue nil
end

What's happening is that the C extension is iterating over the array to eventually dump it out to JSON. It's going through the array in order, appending to the fbuffer as needed. The problem is that that the API extension point of adding a to_json method to a class (or object), without wrapping the code in some sort of begin...rescue , free(buffer), re-raise block results in the buffer never being freed. Normally this isn't too bad, except if a lot of data was appended to the buffer before the error got raised.

To test it against normal behaviour in the above script, take out the offending MyOther.new in the array. It should run much more smoothly :)

Note that since the fbuffers aren't GC marked (not that they should be), it isn't possible to trace this leak using GC.stat.

Once again, not sure if this is a bug or if we should never raise errors from custom to_json methods (ie: always wrap them in a begin... rescue block.

Thanks 😄

[nobu]

Why FBuffer is needed, but not RString?

diff --git i/ext/json/generator/generator.c w/ext/json/generator/generator.c
index a135e28..9632f78 100644
--- i/ext/json/generator/generator.c
+++ w/ext/json/generator/generator.c
@@ -497,6 +497,7 @@ static void State_free(void *ptr)
     if (state->array_delim) fbuffer_free(state->array_delim);
     if (state->object_delim) fbuffer_free(state->object_delim);
     if (state->object_delim2) fbuffer_free(state->object_delim2);
+    if (state->result) fbuffer_free(state->result);
     ruby_xfree(state);
 }

@@ -512,6 +513,7 @@ static size_t State_memsize(const void *ptr)
     if (state->array_delim) size += FBUFFER_CAPA(state->array_delim);
     if (state->object_delim) size += FBUFFER_CAPA(state->object_delim);
     if (state->object_delim2) size += FBUFFER_CAPA(state->object_delim2);
+    if (state->result) size += FBUFFER_CAPA(state->result);
     return size;
 }

@@ -713,7 +715,6 @@ static void generate_json_object(FBuffer *buffer, VALUE Vstate, JSON_Generator_S
     int i, j;
     VALUE key, key_to_s, keys;
     if (max_nesting != 0 && depth > max_nesting) {
-        fbuffer_free(buffer);
         rb_raise(eNestingError, "nesting of %ld is too deep", --state->depth);
     }
     fbuffer_append_char(buffer, '{');
@@ -759,7 +760,6 @@ static void generate_json_array(FBuffer *buffer, VALUE Vstate, JSON_Generator_St
     long depth = ++state->depth;
     int i, j;
     if (max_nesting != 0 && depth > max_nesting) {
-        fbuffer_free(buffer);
         rb_raise(eNestingError, "nesting of %ld is too deep", --state->depth);
     }
     fbuffer_append_char(buffer, '[');
@@ -832,10 +832,8 @@ static void generate_json_float(FBuffer *buffer, VALUE Vstate, JSON_Generator_St
     VALUE tmp = rb_funcall(obj, i_to_s, 0);
     if (!allow_nan) {
         if (isinf(value)) {
-            fbuffer_free(buffer);
             rb_raise(eGeneratorError, "%u: %"PRIsVALUE" not allowed in JSON", __LINE__, RB_OBJ_STRING(tmp));
         } else if (isnan(value)) {
-            fbuffer_free(buffer);
             rb_raise(eGeneratorError, "%u: %"PRIsVALUE" not allowed in JSON", __LINE__, RB_OBJ_STRING(tmp));
         }
     }
@@ -879,7 +877,12 @@ static FBuffer *cState_prepare_buffer(VALUE self)
 {
     FBuffer *buffer;
     GET_STATE(self);
-    buffer = fbuffer_alloc(state->buffer_initial_length);
+    buffer = state->result;
+    if (buffer) {
+        fbuffer_clear(buffer);
+    } else {
+        state->result = buffer = fbuffer_alloc(state->buffer_initial_length);
+    }

     if (state->object_delim) {
         fbuffer_clear(state->object_delim);
diff --git i/ext/json/generator/generator.h w/ext/json/generator/generator.h
index 298c0a4..3cb1990 100644
--- i/ext/json/generator/generator.h
+++ w/ext/json/generator/generator.h
@@ -70,6 +70,7 @@ typedef struct JSON_Generator_StateStruct {
     FBuffer *array_delim;
     FBuffer *object_delim;
     FBuffer *object_delim2;
+    FBuffer *result;
     long max_nesting;
     char allow_nan;
     char ascii_only;

[flori]

Mostly for speed reasons because the fbuffer_* functions can be declared static and -finline-functions can take effect as opposed to Ruby's string methods that are global.