Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

custom function leak #2098

Closed
stefanpenner opened this issue Sep 14, 2017 · 1 comment · Fixed by #2128
Closed

custom function leak #2098

stefanpenner opened this issue Sep 14, 2017 · 1 comment · Fixed by #2128

Comments

@stefanpenner
Copy link
Contributor

stefanpenner commented Sep 14, 2017

versions of stuff

  • NPM version (npm -v): 5.1.0
  • Node version (node -v): v8.1.3
  • Node Process (node -p process.versions):
{ http_parser: '2.7.0',
  node: '8.1.3',
  v8: '5.8.283.41',
  uv: '1.12.0',
  zlib: '1.2.11',
  ares: '1.10.1-DEV',
  modules: '57',
  openssl: '1.0.2l',
  icu: '59.1',
  unicode: '9.0',
  cldr: '31.0.1',
  tz: '2017b' }
  • Node Platform (node -p process.platform): darwin
  • Node architecture (node -p process.arch): x64
  • node-sass version (node -p "require('node-sass').info"):
node-sass	4.5.3	(Wrapper)	[JavaScript]
libsass  	3.5.0.beta.2	(Sass Compiler)	[C/C++

Issue

Using custom functions results in unbounded growth, for example:

const sass = require('.');
function renderSync() {
  sass.renderSync({
    data: '#{headings(2,5)} { color: #08c; }',
    functions: {
      'headings($from: 0, $to: 6)': function(from, to) {
        let f = from.getValue();
        let t = to.getValue();
        let list = new sass.types.List(t - f + 1);

        for (let i = f; i <= t; i++) {
          list.setValue(i - f, new sass.types.String('h' + i));
        }

        return list;
      }
    }
  });
}

while (true) {
  renderSync();
}

When inspecting the process (limiting to only one renderSync it appears \w leak is of arguments and returns values.

renderSync();

using:

export MallocStackLogging=1
node <above-script>
leaks <pid-of-the-above-node-process>
Process:         node [91877]
Path:            /Users/spenner/src/stefanpenner/dotfiles/.config/node/versions/node-v8.1.3-darwin-x64/bin/node
Load Address:    0x100000000
Identifier:      node
Version:         ???
Code Type:       X86-64
Parent Process:  fish [86459]

Date/Time:       2017-09-14 10:58:20.742 -0700
Launch Time:     2017-09-14 10:57:47.137 -0700
OS Version:      Mac OS X 10.12.6 (16G29)
Report Version:  7
Analysis Tool:   /Applications/Xcode.app/Contents/Developer/usr/bin/leaks
Analysis Tool Version:  Xcode 8.3.3 (8E3004b)
----

leaks Report Version:  2.0
Process 91877: 2465 nodes malloced for 689 KB
Process 91877: 5 leaks for 112 total leaked bytes.
Leak: 0x102505c20  size=32  zone: DefaultMallocZone_0x10232b000  length: 26  "headings($from: 0, $to: 6)"
Leak: 0x102505c40  size=32  zone: DefaultMallocZone_0x10232b000  length: 26  "headings($from: 0, $to: 6)"
Leak: 0x10270e0c0  size=16  zone: DefaultMallocZone_0x10232b000  length: 2  "h2"
Leak: 0x10270e450  size=16  zone: DefaultMallocZone_0x10232b000  length: 2  "h4"
Leak: 0x10270e500  size=16  zone: DefaultMallocZone_0x10232b000  length: 2  "h5"

Relevant Malloc traces

  • renderFile + ExtractOptions
    ExtractOptions allocates strings via create_string but does not free them
Leak: 0x101f065d0  size=48  zone: DefaultMallocZone_0x101e27000  length: 34  "eyeglass-fs-exists($absolute-path)"
        Call stack: [thread 0x7ffff04783c0]:
        0x4c93af092a7
        v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*)
        v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>)
        v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))
        Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo<v8::Value> const&)
        render_file(Nan::FunctionCallbackInfo<v8::Value> const&)
        ExtractOptions(v8::Local<v8::Object>, void*, sass_context_wrapper*, bool, bool)
        create_string(v8::MaybeLocal<v8::Value>)
        malloc
  • custom function related:
    pre_process_args appears to allocate SassTypes (and strings within them) but it appears they are not free'd
Leak: 0x101f078d0  size=16  zone: DefaultMallocZone_0x101e27000  length: 4  "Sans"
        Call stack: [thread 0x7ffff04783c0]:
        0x4c93af092a7
        v8::internal::Builtin_HandleApiCallConstruct(int, v8::internal::Object**, v8::internal::Isolate*)
        v8::internal::Builtin_Impl_HandleApiCallConstruct(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*)
        v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))
        Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo<v8::Value> const&)
        SassTypes::SassValueWrapper<SassTypes::String>::New(Nan::FunctionCallbackInfo<v8::Value> const&)
        SassTypes::String::construct(std::vector<v8::Local<v8::Value>, std::allocator<v8::Local<v8::Value> > >, Sass_Value**)
        create_string(v8::MaybeLocal<v8::Value>)
        malloc
@chriseppstein
Copy link
Contributor

chriseppstein commented Sep 14, 2017

It looks like the the string allocated here:

https://github.com/sass/node-sass/blob/master/src/create_string.cpp#L18

in some cases never freed. E.g.

https://github.com/sass/node-sass/blob/master/src/sass_types/string.cpp#L20

The value is passed to libsass where it is copied -- libsass doesn't take ownership of it.

https://github.com/sass/libsass/blob/master/src/sass_values.cpp#L113

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants