Project

General

Profile

Actions

Bug #18154

closed

String#initialize leaks memory for STR_NOFREE strings

Added by peterzhu2118 (Peter Zhu) over 2 years ago. Updated over 2 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:105170]

Description

GitHub PR: https://github.com/ruby/ruby/pull/4814

There is a memory leak in calling the constructor on a string that is marked STR_NOFREE (e.g. a string created from a C string literal). The script below reproduces the memory leak. This is reproducible on all maintained Rubies (2.6.8, 2.7.4, 3.0.2, master) on Ubuntu 20.04.

We create a string marked STR_NOFREE with 0.to_s. to_s for Fixnum has a special optimization for the value 0 (it directly converts it to a C string literal). When we call String#initialize with a capacity it creates a buffer using malloc but does not unset the STR_NOFREE flag. This causes the buffer to be permanently leaked.

100.times do
  1000.times do
    # 0.to_s is a special case that creates a string from a C string literal.
    # https://github.com/ruby/ruby/blob/26153667f91f0c883f6af6b61fac2c0df5312b45/numeric.c#L3393
    # C string literals are always marked STR_NOFREE.
    str = 0.to_s
    # Call String#initialize again to create a buffer with a capacity of 10000
    # characters.
    str.send(:initialize, capacity: 10000)
  end

  # Output the Resident Set Size (memory usage, in KB) of the current Ruby process.
  puts `ps -o rss= -p #{$$}`
end

We can see the leak through the following graph of the Resident Set Size (RSS) comparing the branch vs. master (at commit 26153667f91f0c883f6af6b61fac2c0df5312b45).

Actions #1

Updated by peterzhu2118 (Peter Zhu) over 2 years ago

  • Backport changed from 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN to 2.6: REQUIRED, 2.7: REQUIRED, 3.0: REQUIRED

Updated by Eregon (Benoit Daloze) over 2 years ago

Should it be allowed to even call #initialize on a already-initialized String?
I would think not, for any class.

Doesn't change this is worth fixing though.

Updated by peterzhu2118 (Peter Zhu) over 2 years ago

Indeed, nobody should ever call #initialize on any object more than once. However, making it illegal for calling #initialize multiple times will likely be a breaking change as it's probably a feature used out in the wild.

Actions #4

Updated by peterzhu2118 (Peter Zhu) over 2 years ago

  • Status changed from Open to Closed

Applied in changeset git|5d815542815fe8b939239750bba7f8f0b79c97d6.


[Bug #18154] Fix memory leak in String#initialize

String#initialize can leak memory when called on a string that is marked
with STR_NOFREE because it does not unset the STR_NOFREE flag.

Updated by nagachika (Tomoyuki Chikanaga) over 2 years ago

  • Backport changed from 2.6: REQUIRED, 2.7: REQUIRED, 3.0: REQUIRED to 2.6: REQUIRED, 2.7: REQUIRED, 3.0: DONE

ruby_3_0 650af7d29d98de6a3c2631e31edc6fbe435ece89 merged revision(s) 5d815542815fe8b939239750bba7f8f0b79c97d6.

Updated by usa (Usaku NAKAMURA) over 2 years ago

  • Backport changed from 2.6: REQUIRED, 2.7: REQUIRED, 3.0: DONE to 2.6: REQUIRED, 2.7: DONE, 3.0: DONE

ruby_2_7 d55426f800546cbc3b333ae7ab98c1893f710612 merged revision(s) 5d815542815fe8b939239750bba7f8f0b79c97d6.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0