Oj::Doc.open with a block leaks memory?

[jdelStrother]

Hi there - I noticed what appears to be a memory leak when using Oj::Doc.open with a block.

From the docs it sounds like these two should be equivalent:

Oj::Doc.open(json) { }
Oj::Doc.open(json).close

If I run Oj::Doc.open(json).close in a loop, the memory usage stays stable at 28MB:

$ ruby -e 'require "oj"; json = Oj.dump((1..1000).to_a); 10000.times { Oj::Doc.open(json).close }; puts("RSS: #{`ps -o rss= -p #{Process.pid}`.to_i / 1024}MB")'
RSS: 28MB

If I pass a block and rely on that to auto-close the document, memory usage continues to grow:

$ ruby -e 'require "oj"; json = Oj.dump((1..1000).to_a); 10000.times { Oj::Doc.open(json) { } }; puts("RSS: #{`ps -o rss= -p #{Process.pid}`.to_i / 1024}MB")'
RSS: 446MB

Closing the document within the block does seem to fix the issue:

$ ruby -e 'require "oj"; json = Oj.dump((1..1000).to_a); 10000.times { Oj::Doc.open(json) { |doc| doc.close } }; puts("RSS: #{`ps -o rss= -p #{Process.pid}`.to_i / 1024}MB")'
RSS: 28MB

(Tested on macOS with ruby 3.4.8.)

It looks like fast.c comments out the free-ing code?

oj/ext/oj/fast.c

Lines 1094 to 1099 in 7355146

	// TBD is this needed
	/*
	if (given) {
	OJ_R_FREE(json);
	}
	*/

[ohler55]

If the code is uncommented does the memory leak go away?

[jdelStrother]

Good question, it doesn't. However, this does seem to fix it:

diff --git a/ext/oj/fast.c b/ext/oj/fast.c
index 1ddbe56..bb67205 100644
--- a/ext/oj/fast.c
+++ b/ext/oj/fast.c
@@ -81,6 +81,7 @@ static int   move_step(Doc doc, const char *path, int loc);
 static Leaf  get_doc_leaf(Doc doc, const char *path);
 static Leaf  get_leaf(Leaf *stack, Leaf *lp, const char *path);
 static void  each_value(Doc doc, Leaf leaf);
+static VALUE doc_close(VALUE self);
 
 VALUE oj_doc_class = Qundef;
 
@@ -1146,12 +1147,12 @@ static VALUE doc_open_file(VALUE clas, VALUE filename) {
     fclose(f);
     json[len] = '\0';
     obj       = parse_json(clas, json, given);
-    // TBD is this needed
-    /*
+
     if (given) {
-        OJ_R_FREE(json);
+        doc_close(obj);
+        return Qnil;
     }
-    */
+
     return obj;
 }
 

(I've not done much ruby extension programming, so that may be a naive fix)

[ohler55]

I'll verify with some tracing but that may be the solution.

[ohler55]

I tried something different. Can you try the "no-ostruct" branch.

[jdelStrother]

With no-ostruct, I'm still seeing continuous memory growth with

$ bundle exec ruby -e 'require "oj"; 1000000.times { Oj::Doc.open("[1,2,3]"){} }; IO.popen(%W[ps -o rss= -p #{$$}]) { puts it.read.to_i }'
4041312

It's also crashable if you double-close the document:

$ bundle exec ruby -e 'require "oj"; 100000.times { Oj::Doc.open("[1,2,3]"){it.close} }; IO.popen(%W[ps -o rss= -p #{$$}]) { puts it.read.to_i }'
ruby(44187,0x1fd52e240) malloc: *** error for object 0x600001b7a7c0: pointer being freed was not allocated
ruby(44187,0x1fd52e240) malloc: *** set a breakpoint in malloc_error_break to debug
fish: Job 1, 'command bundle $argv' terminated by signal SIGABRT (Abort)

[meinac]

I think the issue is that the doc and JSON memory are not being freed when a block is provided. We need to call doc_free to free both the doc and the JSON memory allocated in the beginning. We need to do this in this specific place; otherwise, the memory leak still happens in case of an exception.

diff --git a/ext/oj/fast.c b/ext/oj/fast.c
index 1ddbe56..63cd840 100644
--- a/ext/oj/fast.c
+++ b/ext/oj/fast.c
@@ -784,13 +784,8 @@ static VALUE parse_json(VALUE clas, char *json, bool given) {
     result    = rb_protect(protect_open_proc, (VALUE)&pi, &ex);
     if (given || 0 != ex) {
         DATA_PTR(doc->self) = NULL;
-        // TBD is this needed?
-        /*
+
         doc_free(pi.doc);
-        if (0 != ex) {  // will jump so caller will not free
-            OJ_R_FREE(json);
-        }
-        */
     } else {
         result = doc->self;
     }

[Watson1978]

This has been fixed on develop with #1027 (not yet released).