Defining importer function gives Abort trap: 6 error

[browniefed]

By simply defining an importer function an Abort trap: 6 gets thrown whenever I attempt to run anything with an import.

@import "testimport";
.test {
  color: #000;
}

var sass = require('node-sass'),
    path = require('path');

sass.render({
    file: path.resolve(__dirname, './test.scss'),
    outputStyle: 'compressed',
    success: function (css) {
        // the result string
        console.log(css.css);
    },
    error: function(err) {
        console.log(err);
    },
    importer: function(url) {
        return {file: url};
    }
});

[am11]

Thanks for reporting this bug.

I am able to reproduce it. It's actually one of the known issue of this release which is causing the CI build failed as well.

@txdv,

With @browniefed's example and first time using gdb :), I am actually able to narrow it down. It is due to libuv uv_async_send call we have here. It doesn't throw on Windows but on Linux. Having said that, our code is definitely buggy, it is just the OSes initialing process differently (at least this is one bug I have discovered so far.. I have feeling that the synchronous variant is also buggy).

Diagnostic steps:

- `nano /tmp/foo.scss` and paste the following

@import "non-existing-file.scss";
.test {
  color: #000;
}

• mkdir /temp/repos; cd /temp/repos; npm install -g node-gyp.
• git clone https://github.com/sass/node-sass --recursive; cd node-sass; git submodule update --init --recursive; npm install; rm -r vendor/linux*; node-gyp rebuild -d (-d is for debug).
• Have a breakpoint at the aforementioned line.
• Enter node interactive console by entering node and paste the following:

require("./").render({
    file: '/tmp/foo.scss',
    outputStyle: 'compressed',
    success: function (result) {
        // the result string
        console.log(result.css);
    },
    error: function(err) {
        console.log(err);
    },
    importer: function(url) {console.log(url);
        return {file: url};
    }
});

It will return undefined.

• Paste and run the same code again, it will print the error:

{ status: 1,
  file: '/tmp/foo.scss',
  line: 1,
  column: 9,
  message: 'file to import not found or unreadable: testimport\non-existing-file dir: /tmp/',
  code: 1 }

• Run the same the third time and it will terminate the node process with either SegFault or SIGILL.
• If you have a breakpoint here (the method which uv_async_run is supposed to call), it will probably not throw (with indefinite number of tries).

I could not find a way to built this feature with pure "Native abstractions for node.js (nan)" (which is supposed to take care of cross v8 versions), after going through their docs: https://github.com/rvagg/nan/blob/master/README.md. We need to call the NanScope'd function from pure C++ one, to make the callback to JS and get the result from user-defined method asynchronously and pass it back to libsass sequentially (currently we are awaiting using uv's mutices in sass_importer function).

//cc @rvagg, @kkoopa, @mgreter

[am11]

Forgot to mention, you would need to gdb node-sass/build/Debug/binding.node and attach the pid of /usr/bin/node (after entering node interactive console) to the debugger.

[am11]

@mgreter, do we need to free file and prev (which point to sass_importer() params) here without taking over the memory? Or will libsass free those?

[mgreter]

No, either you take the memory (you get a char*) or you get the value and make a copy (you get a const char* on get interfaces). Once you've taken the memory, the ctx will return 0 on further gets! Since it is set to 0 on the ctx, the free call will not free anything! Once you have taken the memory (or made a copy) you need to free that on your own!

Just saw that you were asking about values on your code side. No idea if you need to free them, but if you made a copy or have taken the memory from ctx, you actually do (need to free that on your own)!

[browniefed]

Maybe I'm doing something wrong but I'm still getting the Abort trap error. I cloned your master node-sass repo on your account @am11 , followed the node-gyp rebuild process. Moved the binding.node into the vendor folder. Ran an npm run install and and then tested out a basic importer.
I'm running on Mac OSX Mavericks at the the moment.

[am11]

I'm afk atm. The steps you described have one catch, run npm install and then node-gyp rebuild (won't work if you do the other way around).

-- best
@deel

Sent from my Windows Phone

---

From: Jason Brownmailto:[email protected]
Sent: ‎12/‎28/‎2014 20:10
To: sass/node-sassmailto:[email protected]
Cc: Adeel Mujahidmailto:[email protected]
Subject: Re: [node-sass] Defining importer function gives Abort trap: 6 error (#586)

Maybe I'm doing something wrong but I'm still getting the Abort trap error. I cloned your master node-sass repo on your account @am11 , followed the node-gyp rebuild process. Moved the binding.node into the vendor folder. Ran an npm run install and and then tested out a basic importer.
I'm running on Mac OSX Mavericks at the the moment.

---

Reply to this email directly or view it on GitHub:
#586 (comment)

[browniefed]

The binding.node gets generated after I run node-gyp rebuild. The npm run install is to execute the build scripts, it downloads the binding.node from a github repo if it doesn't exist, if it does exist then it'll using the binding.node that exists in the vendor/darwin-x64 folder thus why I moved it there.
I realize that's unnecessary since it figures out the correct one to use in the lib/index.js.

Regardless I still get the Abort Trap error, I compiled the debug version expecting more output than the abort trap error but still only just that got logged in console.
I'm going to give your debugging steps a shot and see if I can get a more useful error message.

[browniefed]

More info, it appears that renderSync works but then only synchronous importers are allowed, but just a simple render throws the Abort Trap error

[am11]

The only change I made before pushing is to revert back libsass to old commit, because we only update submodule in the repo, when libsass gets a tag release (v3.1.0-beta for instance).
You can try updating the submodule:

cd src/libsass
git remote add libsass https://github.com/sass/libsass
git pull --rebase libsass master
cd ../..
node-gyp rebuild -d

Regarding renderSync and importers, with this change, renderSync will only accept synchronous importer, while render can do both: return result or call done(result) when finish the async job.

[browniefed]

@am11 I tried updating the submodule and still get the Abort trap error.
I understand that renderSync can only do synchronous stuff but I'm saying that sass.renderSync works however calling sass.render with importers throws the Abort Trap 6 error.

This works.

var sass = require('node-sass');

var css = "@import \"non-existing-file\"; \n.test {\ncolor: #000;\n}";

var x = sass.renderSync({
    data: css,
    outputStyle: 'compressed',
    importer: function(url) {
        return {contents: 'div { color: #000; }'};
    }
});

console.log(x);

This does not

var sass = require('node-sass');

var css = "@import \"non-existing-file\"; \n.test {\ncolor: #000;\n}";

sass.render({
    data: css,
    outputStyle: 'compressed',
    success: function(result) {
        console.log(result);
    },
    importer: function(url, prev, done) {
        done({contents: 'div { color: #000; }'});
    }
});

I tried following your debug instructions, I got gdb, but wasn't sure about setting up the project in eclipse so I could debug it, and also wasn't sure how to attach the process to node repl.

[am11]

Tip: if your pwd is node-sass cloned repo's root directory, change this:

var sass = require('node-sass');

to:

var sass = require('./');

Here is what I am getting in Ubuntu 12 with this code:

require("./").render({
    data: "@import \"non-existing-file\"; \n.test {\ncolor: #000;\n}",
    outputStyle: 'compressed',
    success: function(result) {
        console.log(result);
    },
    error: function(error) {
        console.log(error);
    },
    importer: function(url, prev, done) {
        done({contents: 'div { color: #000; }'});
    }
});

Bash:

vagrant@precise64:/temp/node-sass$ pwd
/temp/node-sass
vagrant@precise64:/temp/node-sass$ node
> require("./").render({
...     data: "@import \"non-existing-file\"; \n.test {\ncolor: #000;\n}",
...     outputStyle: 'compressed',
...     success: function(result) {
.....         console.log(result);
.....     },
...     error: function(error) {
.....         console.log(error);
.....     },
...     importer: function(url, prev, done) {
.....         done({contents: 'div { color: #000; }'});
.....     }
... });
undefined
> { css: 'div{color:#000}.test{color:#000}',
  map: '{}',
  stats:
   { entry: 'data',
     start: 1419805648786,
     includedFiles: [ 'non-existing-file' ],
     end: 1419805648787,
     duration: 1 } }

[browniefed]

Sorry my setup may be a little confusing, I just deleted all traces of npm install node-sass and did a git clone into node_modules. Still getting the Abort trap: 6 error. Must be a mac thing?

jasons-mbp:node-sass jasonbrown$ node
> require("./").render({
...     data: "@import \"non-existing-file\"; \n.test {\ncolor: #000;\n}",
...     outputStyle: 'compressed',
...     success: function(result) {
.....         console.log(result);
.....     },
...     error: function(error) {
.....         console.log(error);
.....     },
...     importer: function(url, prev, done) {
.....         done({contents: 'div { color: #000; }'});
.....     }
... });
undefined
> Abort trap: 6

[am11]

Do you get the same result when compile without -d?

cd ~/node-sass/
rm -rf build
node-gyp rebuild
node

I was getting assertion failed followed by node process termination in Windows, after it prints the libsass error:

C:\users\Adeel\Source\Repos\node-sass [master +0 ~1 -0]> node
> require("./").render({
...     data: "@import \"non-existing-file\"; \n.test {\ncolor: #000;\n}",
...     outputStyle: 'compressed',
...     success: function(result) {
.....         console.log(result);
.....     },
...     error: function(error) {
.....         console.log(error);
.....     },
...     importer: function(url, prev, done) {
.....         done({contents: 'div { color: #000; }'});
.....     }
... });
undefined
> { css: 'div{color:#000}.test{color:#000}',
  map: '{}',
  stats:
   { entry: 'data',
     start: 1419808592203,
     includedFiles: [ 'non-existing-file' ],
     end: 1419808598300,
     duration: 6097 } }
Assertion failed: 0, file d:\jenkins\workspace\nodejs-msi\22874dd8\deps\uv\src\win\handle-inl.h, line 158

C:\users\Adeel\Source\Repos\node-sass [master +0 ~1 -0]>

I recompiled without -d, and no more auto termination or assertion failures.. Don't know what to make of it?

[browniefed]

@am11 tested both with -d and without and still get the error on mac.

I did a rebuild on a windows environment and everything works fine there, I haven't tested a vagrant ubuntu environment yet but I imagine it'll work well there. Seems that Mac is the culprit,

[am11]

@browniefed, it's not really the OS thing. The difference in behavior is probably due to the way OSes initiate and manage processes. For instance, npm test is passing on Windows, but after running a very specific stats test, it fails on Linux (https://travis-ci.org/sass/node-sass/jobs/45288685#L599).

Our code has more bugs which need fixing. Currently, I am working on another weird behavior on Linux. Its basically a "manual stress" testing to make this feature bullet proof. For instance, copy these two chunks:

require("./").render({
    file: "test/fixtures/include-files/index.scss",
    outputStyle: 'compressed',
    success: function(result) {
        console.log(result);
    },
    error: function(error) {
        console.log(error);
    },
    importer: function(url, prev, done) {
        done({contents: 'div { color: #000; }'});
    }
});
require("./").render({
    file: "test/fixtures/include-files/index.scss",
    outputStyle: 'compressed',
    success: function(result) {
        console.log(result);
    },
    error: function(error) {
        console.log(error);
    }
});

And paste it like 40-50 times in editor. The copy the whole blob and paste in node.js console. Ubuntu blows with node: ../deps/uv/src/unix/core.c:199: uv__finish_close: Assertion```0' failed.\nAborted after running few, Windows passes all. Both are compiled with Release configuration. So far windows passes like 4000+ lines of codes, ran in the same manner. I was doing it to detect memory leaks (poor man's way of doing it, as @mgreter once put it 😄 ). The expert-mode way is to use something like Valgrind, which begs some major motivation.

So we have like five moving parts here, OS, libuv, v8, nan and libsass. And the sixth one is our binding code gluing them together! All we need is to make sure that all of them are on board, which is bit painful, when you get some gibberish thrown by debugger from memory heaps.

After reading this: http://stackoverflow.com/a/5856959/863980, I think I am going to try valgrind soon! Hopefully we will get somewhere. The catch is: if we do valgrind node, and then the actual script run, we would probably need to filter out the node internal memory logs and false-positives (as mentioned under caveats), but better than going nowhere (and pulling the hairs out, I am getting the exact same thing with gdb on Ubuntu)!

[am11]

I am getting the exact same thing with gdb on Ubuntu

to expand on this:

cd into/cloned/dir/of/node-sass
gdb node
run ./node_modules/.bin/mocha test
# where test is a directory containing test

GDB will throw the following on test failure:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6c123b7 in kill () from /lib/x86_64-linux-gnu/libc.so.6

this happens after running the test should provide a start timestamp, which mocha suggests is passing. The next test ('s it() block) doesn't hit.

[browniefed]

Ah wow I was unaware the current environment was such a mish-mash of libraries. I wish I could be of more assitance, I'm not exactly skilled in the ways of C++ development but I really appreciate all this work you're putting in to get this figured out. The whole processing of importers concept is great and powerful and should make extending sass without ruby a breeze.

[am11]

On Ubuntu, https://wiki.ubuntu.com/Valgrind made it super easy to configure valgrind and detect memory leaks:

sudo apt-get install valgrind

# mkdir /temp
# git clone node-sass https://github.com/am11/node-sass --recursive
# cd /temp/node-sass
# git submodule --recursive --init
# npm install
# node-gyp rebuild -d

G_SLICE=always-malloc G_DEBUG=gc-friendly  valgrind -v --tool=memcheck --leak-check=full --show-reachable=yes --num-callers=40 --log-file=valgrind.log $(which node) node_modules/.bin/mocha test

Will produce valgrind.log in current dir.
//cc @mgreter

[mgreter]

@am11: First let me be clear: I really think the problem is not in libsass but with (the usage of) libuv. Next: I don't know shit about libuv but tried to hunt down your nasty race condition anyway. So here my uneducated guesses:

The error can also be avoided if this line in sass_free_context_wrapper gets commented out: free(ctx_w);. But I also don't see why this should error out (it seems valid to deallocate the memory here). Next observation was that commenting out uv_async_init ... in binding.cpp also "avoids" the crash. I don't know any details about libuv, but I think I know what it should be doing to get this working in an async manner. So I started to wonder where you actually "terminate" the "intermediate" thread. The google search on this lead me to the conclusion that you may miss some uv_closecalls ...

I guess that's as much as I want to dive into the whole libuv stuff. But I'm pretty convinced that the cluprit is somewhere in there. But I also could be totally wrong here!

As a side note: You should be aware that uv_work_t, uv_mutex_t, uv_cond_t and uv_async_t are all direct members to (sass_context_wrapper) ctx_w. You seem to pass a lot of pointers to these structs (which is valid) to the internal functions of libuv; ie. &ctx_w->async. Once you free ctx_w, these pointers will become wild pointers!

[am11]

@mgreter, thanks for looking into it.

You are right about uv_close, and this was the first thing which @txdv pointed out. I had that fixed with this commit: am11@35a82ab#diff-672b939c4bdf28c42fc7ba5da56105dbR240.

Now with the tip of my master branch, I generated two different sets of logs on Ubuntu:

Valgrind: https://gist.github.com/am11/6cfcebdef05e3751f16d#file-valgrind-log-L151
strace: https://gist.github.com/am11/ee8e8e1fa2bf39516c15#file-strace-mocha-log-L7680

Both captured the SIGSEGV thrown after running a specific stat-related test and free()'ing the memory.

At libuv IRC channel, someone suggested me to change calloc call with C++ style memory allocation (sass_context_wrapper* ctx_w = new sass_context_wrapper;) and then in dtor, use C++ style delete to deallocate ctx_w, since C++ takes ref tracking into account (save the masses from double-free()'ing and considered luxury over C in this regard). But it did not work in this scenario, as libsass require us to pass C-style pointer(s) and uses free() to deallocate them, which C++ pointers on free() throw 'invalid pointer' (at least with glibc).

There is another odd behavior i'm facing when dealing with CLI. I think they might be related (implicitly?). Besides having uv_async_send, uv_cond_wait, uv_mutex, it seems like libuv require some kind of a "push" or another signal to run. According to the docs, uv_async_send runs at least once! But in this case either the thread is sleeping or there is some kind of a deadlock.

[kkoopa]

You should only free the memory from the uv_close callback.

void uv_close(uv_handle_t* handle, uv_close_cb close_cb)

Can for example be done with uv_close((uv_handle_t*)&ctx_w->async, free);

[am11]

@kkoopa, thanks for your advice. I have applied this patch locally: https://gist.github.com/am11/6b227670e5a827ed2c15.
However, the status of npm tests on Ubuntu is same (segmentation fault after running first stats test and just before starting the next one).

[am11]

Hello @browniefed, the TravisCI build is now passing and we will release v2.0.0 🔜'ish.

I can confirm that the build is pretty ok on both Linux and Windows.

Could you please give it another try (with my master branch) on OSX and confirm if the issue is fixed there? :)

[browniefed]

No go. Still getting the abort trap error. 😢
My process

git clone --recursive https://github.com/am11/node-sass.git
cd node-sass
git submodule update --init --recursive
npm install
npm install -g node-gyp
node-gyp rebuild -d //Tried both with and without

node

paste

require("./").render({
    data: "@import \"non-existing-file\"; \n.test {\ncolor: #000;\n}",
    outputStyle: 'compressed',
    success: function(result) {
        console.log(result);
    },
    error: function(error) {
        console.log(error);
    },
    importer: function(url, prev, done) {
        done({contents: 'div { color: #000; }'});
    }
});

> require("./").render({
...     data: "@import \"non-existing-file\"; \n.test {\ncolor: #000;\n}",
...     outputStyle: 'compressed',
...     success: function(result) {
.....         console.log(result);
.....     },
...     error: function(error) {
.....         console.log(error);
.....     },
...     importer: function(url, prev, done) {
.....         done({contents: 'div { color: #000; }'});
.....     }
... });
undefined
> Abort trap: 6

In attempting to be useful I'm running Mac OSX 10.9 Mavericks so luckily valgrind works on it. I got it installed and ran it with the commands you had up above. I get to the importer tests and everything then it just sits there doing nothing.

https://gist.github.com/browniefed/cad5225b63f02b747e24
Ran it again for good measure and got the same results

https://gist.github.com/browniefed/3a1c271637631855351e

[am11]

@browniefed, thanks for the details. Please show the output of node -p process.versions.

On Linux and Windows, it is giving me this result:

require("./").render({
...     data: "@import \"non-existing-file\"; \n.test {\ncolor: #000;\n}",
...     outputStyle: 'compressed',
...     success: function(result) {
.....         console.log(result);
.....     },
...     error: function(error) {
.....         console.log(error);
.....     },
...     importer: function(url, prev, done) {
.....         done({contents: 'div { color: #000; }'});
.....     }
... });
undefined
> { css: 'div{color:#000}.test{color:#000}',
  map: '{}',
  stats:
   { entry: 'data',
     start: 1420517121859,
     includedFiles: [ 'non-existing-file' ],
     end: 1420517121861,
     duration: 2 } }

[browniefed]

node -p process.versions
{ http_parser: '1.0',
  node: '0.10.35',
  v8: '3.14.5.9',
  ares: '1.9.0-DEV',
  uv: '0.10.30',
  zlib: '1.2.8',
  modules: '11',
  openssl: '1.0.1j' }

[browniefed]

If there is nothing useful in those valgrind logs I'll run any other valgrind stuff (i.e. not running it against mocha tests, etc). Just let me know

[rvagg]

I'm very sorry that I don't have the bandwidth to digest or continue with this issue but I do want to point to my earlier comments @ #576 (comment) which I still stand by and suspect might be related to the problems here. I'm unsubscribing from this thread but feel free to rope me back in by @-mention if you have something concrete for me to discuss. You should also lean on @koopa's expertise here if you can.

[am11]

Our requirement is bit different and unfortunately that cannot be accomplished by the pure-nan solution. Here is the proof-of-concept how it would look like in pure-nan terms: b2da1c7. But it throws exception at NanAsyncWorker ctor, when nan tries to run NanScope() on the libsass' parser thread (in case of ctx_w->importer_callback). We were facing this exact issue with double lock (our first approach), and hence we had to use uv_async_send.

Perhaps it is possible with something other than NanAsyncWorker, which I couldn't figure out from examples and elaborated docs.

[am11]

@browniefed, I do not have Mac and unfortunately I cannot test darwin binary at this point. So v2.0.0 would probably not be stable for Mac. We will keep this issue open, so in future, anyone with OSX can debug and spot what is wrong there.

Nonetheless, we are at a much better place where we were 10 days ago. So thanks again for reporting this issue! :)

[am11]

This is fixed by 0720fa8 and ff53926 via #615.
All tests are passing on Mac now.
@browniefed confirmed it!

Once again, thanks for everyone who helped us pulling this one off. 👍 🎉

[callumlocke]

I just updated to the latest node-sass master, but I still get Abort trap: 6 when I define an importer function. On OS X 10.9.5.

[browniefed]

@callumlocke could you say anything about the process? Did you build custom, or did you let it install the binaries off of node-sass-binaries?

[callumlocke]

I just did git pull to get the latest code, then npm install. I didn't do
anything custom.

It works fine until I define an importer function.
On Mon, 19 Jan 2015 at 17:29, Jason Brown [email protected] wrote:

@callumlocke https://github.com/callumlocke could you say anything
about the process? Did you build custom, or did you let it install the
binaries off of node-sass-binaries?

—
Reply to this email directly or view it on GitHub
#586 (comment).

[browniefed]

@callumlocke please follow this

Rebuilding binaries

Node-sass includes pre-compiled binaries for popular platforms, to add a binary for your platform follow these steps:

Check out the project:

git clone --recursive https://github.com/sass/node-sass.git
cd node-sass
git submodule update --init --recursive
npm install
npm install -g node-gyp
node-gyp rebuild  # to make debug release, use -d switch

[callumlocke]

awesome, thanks