Fix segfaults (testing)

lib/index.js

@@ -65,14 +65,12 @@ function getStats(options) {
 /**
  * End stats
  *
- * @param {Object} options
+ * @param {Object} stats
  * @param {Object} sourceMap
  * @api private
  */
 
-function endStats(options) {
-  var stats = options.stats || {};
-
+function endStats(stats) {
   stats.end = Date.now();
   stats.duration = stats.end - stats.start;
 
@@ -144,8 +142,6 @@ function getOptions(options) {
     throw new Error('`imagePath` needs to be a string');
   }
 
-  options.stats = getStats(options);
-
   var error = options.error;
   var success = options.success;
   var importer = options.importer;
@@ -164,16 +160,16 @@ function getOptions(options) {
     }
   };
 
-  options.success = function(css, sourceMap) {
-    sourceMap = JSON.parse(sourceMap);
+  options.success = function() {
+    options.result.sourceMap = JSON.parse(options.result.sourceMap);
 
-    endStats(options, sourceMap);
+    var stats = endStats(options.result.stats);
 
     if (success) {
       success({
-        css: css,
-        map: sourceMap,
-        stats: options.stats
+        css: options.result.css,
+        map: options.result.sourceMap,
+        stats: stats
       });
     }
   };
@@ -195,6 +191,10 @@ function getOptions(options) {
   delete options.source_comments;
   delete options.sourceComments;
 
+  options.result = {
+    stats: getStats(options)
+  };
+
   return options;
 }
 
@@ -213,6 +213,7 @@ var binding = require(getBinding());
 
 module.exports.render = function(options) {
   options = getOptions(options);
+
   options.data ? binding.render(options) : binding.renderFile(options);
 };
 
@@ -224,23 +225,15 @@ module.exports.render = function(options) {
  */
 
 module.exports.renderSync = function(options) {
-  var output;
-
   options = getOptions(options);
-  output = options.data ? binding.renderSync(options) : binding.renderFileSync(options);
-
-  endStats(options);
 
-  var result = {
-    css: output,
-    map: options.stats.sourceMap
-  };
+  var result = options.data ? binding.renderSync(options) : binding.renderFileSync(options);
 
-  delete options.stats.sourceMap;
+  if(result) {
+    options.result.stats = endStats(options.result.stats);
 
-  result.stats = options.stats;
-
-  return result;
+    return options.result;
+  }
 };
 
 /**

src/binding.cpp

@@ -15,7 +15,7 @@ char* CreateString(Local<Value> value) {
 
 std::vector<sass_context_wrapper*> imports_collection;
 
-void dispatched_async_uv_callback(uv_async_t *req){
+void dispatched_async_uv_callback(uv_async_t *req) {
   NanScope();
   sass_context_wrapper* ctx_w = static_cast<sass_context_wrapper*>(req->data);
 
@@ -24,8 +24,8 @@ void dispatched_async_uv_callback(uv_async_t *req){
   imports_collection.push_back(ctx_w);
 
   Handle<Value> argv[] = {
-    NanNew<String>(strdup(ctx_w->file)),
-    NanNew<String>(strdup(ctx_w->prev)),
+    NanNew<String>(strdup(ctx_w->file ? ctx_w->file : 0)),
+    NanNew<String>(strdup(ctx_w->prev ? ctx_w->prev : 0)),
     NanNew<Number>(imports_collection.size() - 1)
   };
 
@@ -40,8 +40,8 @@ struct Sass_Import** sass_importer(const char* file, const char* prev, void* coo
 {
   sass_context_wrapper* ctx_w = static_cast<sass_context_wrapper*>(cookie);
 
-  ctx_w->file = strdup(file);
-  ctx_w->prev = strdup(prev);
+  ctx_w->file = file ? strdup(file) : 0;
+  ctx_w->prev = prev ? strdup(prev) : 0;
   ctx_w->async.data = (void*)ctx_w;
   uv_async_send(&ctx_w->async);
 
@@ -52,7 +52,7 @@ struct Sass_Import** sass_importer(const char* file, const char* prev, void* coo
      */
     uv_cond_wait(&ctx_w->importer_condition_variable, &ctx_w->importer_mutex);
   }
-  else{
+  else {
     /*  that is sync: RenderSync() or RenderFileSync,
      *  we need to explicitly uv_run as the event loop
      *  is blocked; waiting down the chain.
@@ -66,6 +66,8 @@ struct Sass_Import** sass_importer(const char* file, const char* prev, void* coo
 void ExtractOptions(Local<Object> options, void* cptr, sass_context_wrapper* ctx_w, bool isFile, bool isSync) {
   struct Sass_Context* ctx;
 
+  NanAssignPersistent(ctx_w->result, options->Get(NanNew("result"))->ToObject());
+
   if (isFile) {
     ctx = sass_file_context_get_context((struct Sass_File_Context*) cptr);
     ctx_w->fctx = (struct Sass_File_Context*) cptr;
@@ -78,8 +80,6 @@ void ExtractOptions(Local<Object> options, void* cptr, sass_context_wrapper* ctx
   struct Sass_Options* sass_options = sass_context_get_options(ctx);
 
   if (!isSync) {
-    NanAssignPersistent(ctx_w->stats, options->Get(NanNew("stats"))->ToObject());
-
     ctx_w->request.data = ctx_w;
 
     // async (callback) style
@@ -113,7 +113,7 @@ void ExtractOptions(Local<Object> options, void* cptr, sass_context_wrapper* ctx
   sass_option_set_precision(sass_options, options->Get(NanNew("precision"))->Int32Value());
 }
 
-void FillStatsObj(Handle<Object> stats, Sass_Context* ctx) {
+void GetStats(Handle<Object> result, Sass_Context* ctx) {
   char** included_files = sass_context_get_included_files(ctx);
   Handle<Array> arr = NanNew<Array>();
 
@@ -123,8 +123,10 @@ void FillStatsObj(Handle<Object> stats, Sass_Context* ctx) {
     }
   }
 
-  (*stats)->Set(NanNew("includedFiles"), arr);
+  (*result)->Get(NanNew("stats"))->ToObject()->Set(NanNew("includedFiles"), arr);
+}
 
+void GetSourceMap(Handle<Object> result, Sass_Context* ctx) {
   Handle<Value> source_map;
 
   if (sass_context_get_error_status(ctx)) {
@@ -138,7 +140,19 @@ void FillStatsObj(Handle<Object> stats, Sass_Context* ctx) {
     source_map = NanNew<String>("{}");
   }
 
-  (*stats)->Set(NanNew("sourceMap"), source_map);
+  (*result)->Set(NanNew("sourceMap"), source_map);
+}
+
+int GetResult(Handle<Object> result, Sass_Context* ctx) {
+  int status = sass_context_get_error_status(ctx);
+
+  if (status == 0) {
+    (*result)->Set(NanNew("css"), NanNew<String>(sass_context_get_output_string(ctx)));
+    GetStats(result, ctx);
+    GetSourceMap(result, ctx);
+  }
+
+  return status;
 }
 
 void make_callback(uv_work_t* req) {
@@ -146,35 +160,27 @@ void make_callback(uv_work_t* req) {
 
   TryCatch try_catch;
   sass_context_wrapper* ctx_w = static_cast<sass_context_wrapper*>(req->data);
-  int error_status;
   struct Sass_Context* ctx;
 
   if (ctx_w->dctx) {
     ctx = sass_data_context_get_context(ctx_w->dctx);
-    FillStatsObj(NanNew(ctx_w->stats), ctx);
-    error_status = sass_context_get_error_status(ctx);
   }
   else {
     ctx = sass_file_context_get_context(ctx_w->fctx);
-    FillStatsObj(NanNew(ctx_w->stats), ctx);
-    error_status = sass_context_get_error_status(ctx);
   }
 
-  if (error_status == 0) {
+  int status = GetResult(ctx_w->result, ctx);
+
+  if (status == 0) {
     // if no error, do callback(null, result)
-    const char* val = sass_context_get_output_string(ctx);
-    Local<Value> argv[] = {
-      NanNew<String>(val),
-      NanNew(ctx_w->stats)->Get(NanNew("sourceMap"))
-    };
-    ctx_w->success_callback->Call(2, argv);
+    ctx_w->success_callback->Call(0, 0);
   }
   else {
     // if error, do callback(error)
     const char* err = sass_context_get_error_json(ctx);
     Local<Value> argv[] = {
       NanNew<String>(err),
-      NanNew<Integer>(error_status)
+      NanNew<Integer>(status)
     };
     ctx_w->error_callback->Call(2, argv);
   }
@@ -213,21 +219,22 @@ NAN_METHOD(RenderSync) {
 
   ExtractOptions(options, dctx, ctx_w, false, true);
   compile_data(dctx);
-  FillStatsObj(options->Get(NanNew("stats"))->ToObject(), ctx);
 
-  if (sass_context_get_error_status(ctx) == 0) {
-    Local<String> output = NanNew<String>(sass_context_get_output_string(ctx));
+  int result = GetResult(ctx_w->result, ctx);
+  Local<String> error;
 
-    sass_delete_data_context(dctx);
-    NanReturnValue(output);
+  if (result != 0) {
+    error = NanNew<String>(sass_context_get_error_json(ctx));
   }
 
-  Local<String> error = NanNew<String>(sass_context_get_error_json(ctx));
-
   sass_free_context_wrapper(ctx_w);
+  free(source_string);
+
+  if (result != 0) {
     NanThrowError(error);
+  }
 
-  NanReturnUndefined();
+  NanReturnValue(NanNew<Boolean>(result == 0));
 }
 
 NAN_METHOD(RenderFile) {
@@ -259,22 +266,22 @@ NAN_METHOD(RenderFileSync) {
 
   ExtractOptions(options, fctx, ctx_w, true, true);
   compile_file(fctx);
-  FillStatsObj(options->Get(NanNew("stats"))->ToObject(), ctx);
-  free(input_path);
 
-  if (sass_context_get_error_status(ctx) == 0) {
-    Local<String> output = NanNew<String>(sass_context_get_output_string(ctx));
+  int result = GetResult(ctx_w->result, ctx);
+  Local<String> error;
 
-    sass_delete_file_context(fctx);
-    NanReturnValue(output);
+  if (result != 0) {
+    error = NanNew<String>(sass_context_get_error_json(ctx));
   }
 
-  Local<String> error = NanNew<String>(sass_context_get_error_json(ctx));
-
   sass_free_context_wrapper(ctx_w);
+  free(input_path);
+
+  if (result != 0) {
     NanThrowError(error);
+  }
 
-  NanReturnUndefined();
+  NanReturnValue(NanNew<Boolean>(result == 0));
 }
 
 NAN_METHOD(ImportedCallback) {
@@ -283,9 +290,7 @@ NAN_METHOD(ImportedCallback) {
   TryCatch try_catch;
 
   Local<Object> options = args[0]->ToObject();
-  char* source_string = CreateString(options->Get(NanNew("index")));
   Local<Value> returned_value = options->Get(NanNew("objectLiteral"));
-
   size_t index = options->Get(NanNew("index"))->Int32Value();
 
   if (index >= imports_collection.size()) {

libsass @ 9852e41a

-Subproject commit 7aaa45a979ce65d50b3158ff50bf9409f8955063
+Subproject commit 9852e41a11f16e8d52992a0f6e4d08068e9cd0ad

src/sass_context_wrapper.cpp

@@ -31,24 +31,23 @@ extern "C" {
 
   void sass_free_context_wrapper(sass_context_wrapper* ctx_w) {
     if (ctx_w->dctx) {
-      sass_delete_data_context(ctx_w->dctx);
+    //  sass_delete_data_context(ctx_w->dctx);
     }
     else if (ctx_w->fctx) {
       sass_delete_file_context(ctx_w->fctx);
     }
 
-    NanDisposePersistent(ctx_w->stats);
+    NanDisposePersistent(ctx_w->result);
 
     delete ctx_w->success_callback;
     delete ctx_w->error_callback;
     delete ctx_w->importer_callback;
     delete ctx_w->file;
     delete ctx_w->prev;
-    delete ctx_w->cookie;
 
     uv_mutex_destroy(&ctx_w->importer_mutex);
     uv_cond_destroy(&ctx_w->importer_condition_variable);
 
-    free(ctx_w);
+    // free(ctx_w);
   }
 }

src/sass_context_wrapper.h

@@ -14,7 +14,7 @@ extern "C" {
   struct sass_context_wrapper {
     Sass_Data_Context* dctx;
     Sass_File_Context* fctx;
-    Persistent<Object> stats;
+    Persistent<Object> result;
     uv_work_t request;
     uv_mutex_t importer_mutex;
     uv_cond_t importer_condition_variable;

test/api.js

@@ -8,7 +8,19 @@ var assert = require('assert'),
 
 describe('api', function() {
   describe('.render(options)', function() {
-    it('should compile sass to css', function(done) {
+    it('should compile sass to css with file', function(done) {
+      var expected = read(fixture('simple/expected.css'), 'utf8').trim();
+
+      sass.render({
+        file: fixture('simple/index.scss'),
+        success: function(result) {
+          assert.equal(result.css.trim(), expected.replace(/\r\n/g, '\n'));
+          done();
+        }
+      });
+    });
+
+    it('should compile sass to css with data', function(done) {
       var src = read(fixture('simple/index.scss'), 'utf8');
       var expected = read(fixture('simple/expected.css'), 'utf8').trim();
 
@@ -104,19 +116,6 @@ describe('api', function() {
       });
     });
 
-    it('should compile with stats', function(done) {
-      var src = fixture('precision/index.scss');
-
-      sass.render({
-        file: src,
-        sourceMap: true,
-        success: function(result) {
-          assert.equal(result.stats.entry, src);
-          done();
-        }
-      });
-    });
-
     it('should contain all included files in stats when data is passed', function(done) {
       var src = fixture('include-files/index.scss');
       var expected = [
@@ -134,7 +133,7 @@ describe('api', function() {
       });
     });
 
-    it('should override imports with custom importer', function(done) {
+    it('should override imports with custom importer with data', function(done) {
       var src = read(fixture('include-files/index.scss'), 'utf8');
 
       sass.render({
@@ -151,10 +150,34 @@ describe('api', function() {
         }
       });
     });
+
+    it('should override imports with custom importer with file', function(done) {
+      sass.render({
+        file: fixture('include-files/index.scss'),
+        success: function(result) {
+          assert.equal(result.css.trim(), 'div {\n  color: yellow; }\n\ndiv {\n  color: yellow; }');
+          done();
+        },
+        importer: function(url, prev, done) {
+          done({
+            file: '/some/other/path.scss',
+            contents: 'div {color: yellow;}'
+          });
+        }
+      });
+    });
   });
 
   describe('.renderSync(options)', function() {
-    it('should compile sass to css', function(done) {
+    it('should compile sass to css with file', function(done) {
+      var expected = read(fixture('simple/expected.css'), 'utf8').trim();
+      var result = sass.renderSync({file: fixture('simple/index.scss')});
+
+      assert.equal(result.css.trim(), expected.replace(/\r\n/g, '\n'));
+      done();
+    });
+
+    it('should compile sass to css with data', function(done) {
       var src = read(fixture('simple/index.scss'), 'utf8');
       var expected = read(fixture('simple/expected.css'), 'utf8').trim();
       var result = sass.renderSync({data: src});
@@ -183,7 +206,7 @@ describe('api', function() {
       done();
     });
 
-    it('should override imports with custom importer', function(done) {
+    it('should override imports with custom importer with data', function(done) {
       var src = read(fixture('include-files/index.scss'), 'utf8');
 
       var result = sass.renderSync({
@@ -199,6 +222,21 @@ describe('api', function() {
       assert.equal(result.css.trim(), 'div {\n  color: yellow; }\n\ndiv {\n  color: yellow; }');
       done();
     });
+
+    it('should override imports with custom importer with file', function(done) {
+      var result = sass.renderSync({
+        file: fixture('include-files/index.scss'),
+        importer: function(url, prev, finish) {
+          finish({
+            file: '/some/other/path.scss',
+            contents: 'div {color: yellow;}'
+          });
+        }
+      });
+
+      assert.equal(result.css.trim(), 'div {\n  color: yellow; }\n\ndiv {\n  color: yellow; }');
+      done();
+    });
   });
 
   describe('.render({stats: {}})', function() {

spec @ be28a5a4

-Subproject commit 074ae3b24bfc3ff19a98efdec02415ed6fd5759a
+Subproject commit be28a5a4dbfb8b6bc137609733ca46eed3d6b3aa