Skip to content

Commit

Permalink
Fix memory leaks found by ruby_memcheck
Browse files Browse the repository at this point in the history
See #104

Specifically, ensure we delete the previous input inside an RE2::Scanner
before replacing it and check whether inputs are strings as early as
possible to avoid raising an exception after allocating memory.

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 23fab86 commit 60e3d3f
Showing 1 changed file with 20 additions and 0 deletions.
20 changes: 20 additions & 0 deletions ext/re2/re2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ static VALUE re2_scanner_rewind(VALUE self) {
re2_scanner *c;
Data_Get_Struct(self, re2_scanner, c);

delete c->input;
c->input = new(std::nothrow) re2::StringPiece(StringValuePtr(c->text));
c->eof = false;

Expand Down Expand Up @@ -785,6 +786,10 @@ static VALUE re2_regexp_initialize(int argc, VALUE *argv, VALUE self) {
re2_pattern *p;

rb_scan_args(argc, argv, "11", &pattern, &options);

/* Ensure pattern is a string. */
StringValue(pattern);

Data_Get_Struct(self, re2_pattern, p);

if (RTEST(options)) {
Expand Down Expand Up @@ -1341,6 +1346,9 @@ static VALUE re2_regexp_match_p(const VALUE self, VALUE text) {
* c = RE2::Regexp.new('(\w+)').scan("Foo bar baz")
*/
static VALUE re2_regexp_scan(const VALUE self, VALUE text) {
/* Ensure text is a string. */
StringValue(text);

re2_pattern *p;
re2_scanner *c;

Expand Down Expand Up @@ -1382,6 +1390,9 @@ static VALUE re2_regexp_scan(const VALUE self, VALUE text) {
*/
static VALUE re2_Replace(VALUE, VALUE str, VALUE pattern,
VALUE rewrite) {
/* Ensure rewrite is a string. */
StringValue(rewrite);

re2_pattern *p;

/* Take a copy of str so it can be modified in-place by
Expand All @@ -1397,6 +1408,9 @@ static VALUE re2_Replace(VALUE, VALUE str, VALUE pattern,
return encoded_str_new(str_as_string.data(), str_as_string.size(),
p->pattern->options().encoding());
} else {
/* Ensure pattern is a string. */
StringValue(pattern);

RE2::Replace(&str_as_string, StringValuePtr(pattern),
StringValuePtr(rewrite));

Expand All @@ -1422,6 +1436,9 @@ static VALUE re2_Replace(VALUE, VALUE str, VALUE pattern,
*/
static VALUE re2_GlobalReplace(VALUE, VALUE str, VALUE pattern,
VALUE rewrite) {
/* Ensure rewrite is a string. */
StringValue(rewrite);

/* Take a copy of str so it can be modified in-place by
* RE2::GlobalReplace.
*/
Expand All @@ -1436,6 +1453,9 @@ static VALUE re2_GlobalReplace(VALUE, VALUE str, VALUE pattern,
return encoded_str_new(str_as_string.data(), str_as_string.size(),
p->pattern->options().encoding());
} else {
/* Ensure pattern is a string. */
StringValue(pattern);

RE2::GlobalReplace(&str_as_string, StringValuePtr(pattern),
StringValuePtr(rewrite));

Expand Down

0 comments on commit 60e3d3f

Please sign in to comment.