Skip to content

Commit

Permalink
Fix leak in RE2::Set#add
Browse files Browse the repository at this point in the history
See #104

When we raise an exception in re2_set_add, the memory used by the
std::string used to store the error message is never freed. Fix this by
ensuring it goes out of scope before we call rb_raise.

However, we also need a copy of what is inside it to return to the user
so we take a copy of its contents as a C string first. The current
longest error message inside RE2 is 35 characters long so 100 characters
gives us some headroom in case new releases of RE2 add longer messages.

Thanks to @peterzhu2118 for both authoring ruby_memcheck and helping
find the source of these leaks.
  • Loading branch information
mudge committed Sep 22, 2023
1 parent 60e3d3f commit befe65b
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions ext/re2/re2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1583,13 +1583,23 @@ static VALUE re2_set_initialize(int argc, VALUE *argv, VALUE self) {
static VALUE re2_set_add(VALUE self, VALUE pattern) {
StringValue(pattern);
re2::StringPiece regex(RSTRING_PTR(pattern), RSTRING_LEN(pattern));
std::string err;
re2_set *s;
Data_Get_Struct(self, re2_set, s);

int index = s->set->Add(regex, &err);
/* To prevent the memory of the err string leaking when we call rb_raise,
* take a copy of it and let it go out of scope.
*/
char msg[100];
int index;

{
std::string err;
index = s->set->Add(regex, &err);
strncpy(msg, err.c_str(), sizeof(msg));
}

if (index < 0) {
rb_raise(rb_eArgError, "str rejected by RE2::Set->Add(): %s", err.c_str());
rb_raise(rb_eArgError, "str rejected by RE2::Set->Add(): %s", msg);
}

return INT2FIX(index);
Expand Down

0 comments on commit befe65b

Please sign in to comment.