Fix memory leaks and improve input_source handling

Author: mgreter

context.cpp

@@ -58,7 +58,8 @@ namespace Sass {
     c_options               (initializers.c_options()),
     c_compiler              (initializers.c_compiler()),
     source_c_str            (initializers.source_c_str()),
-    sources                 (vector<const char*>()),
+    sources                 (vector<char*>()),
+    strings                 (vector<char*>()),
     plugin_paths            (initializers.plugin_paths()),
     include_paths           (initializers.include_paths()),
     queue                   (vector<Sass_Queued>()),
@@ -148,12 +149,16 @@ namespace Sass {
 
   Context::~Context()
   {
-    // everything that gets put into sources will be freed by us
-    for (size_t n = 0; n < import_stack.size(); ++n) sass_delete_import(import_stack[n]);
+    // make sure we free the source even if not processed!
+    if (sources.size() == 0 && source_c_str) free(source_c_str);
     // sources are allocated by strdup or malloc (overtaken from C code)
-    for (size_t i = 0; i < sources.size(); ++i) free((void*)sources[i]);
-    // clear inner structures (vectors)
-    sources.clear(); import_stack.clear();
+    for (size_t i = 0; i < sources.size(); ++i) free(sources[i]);
+    // free all strings we kept alive during compiler execution
+    for (size_t n = 0; n < strings.size(); ++n) free(strings[n]);
+    // everything that gets put into sources will be freed by us
+    for (size_t m = 0; m < import_stack.size(); ++m) sass_delete_import(import_stack[m]);
+    // clear inner structures (vectors) and input source
+    sources.clear(); import_stack.clear(); source_c_str = 0;
   }
 
   void Context::setup_color_map()
@@ -245,7 +250,7 @@ namespace Sass {
       }
     }
   }
-  void Context::add_source(string load_path, string abs_path, const char* contents)
+  void Context::add_source(string load_path, string abs_path, char* contents)
   {
     sources.push_back(contents);
     included_files.push_back(abs_path);
@@ -317,8 +322,10 @@ namespace Sass {
         0, 0
       );
       import_stack.push_back(import);
-      const char* path = sass_strdup(queue[i].abs_path.c_str());
-      Parser p(Parser::from_c_str(queue[i].source, *this, ParserState(path, queue[i].source, i)));
+      // keep a copy of the path around (for parser states)
+      strings.push_back(sass_strdup(queue[i].abs_path.c_str()));
+      ParserState pstate(strings.back(), queue[i].source, i);
+      Parser p(Parser::from_c_str(queue[i].source, *this, pstate));
       Block* ast = p.parse();
       sass_delete_import(import_stack.back());
       import_stack.pop_back();
@@ -369,7 +376,7 @@ namespace Sass {
     if(is_indented_syntax_src) {
       char * contents = sass2scss(source_c_str, SASS2SCSS_PRETTIFY_1 | SASS2SCSS_KEEP_COMMENT);
       add_source(input_path, input_path, contents);
-      delete [] source_c_str;
+      free(source_c_str);
       return parse_file();
     }
     add_source(input_path, input_path, source_c_str);

context.hpp

@@ -37,11 +37,13 @@ namespace Sass {
 
     struct Sass_Options* c_options;
     struct Sass_Compiler* c_compiler;
-    const char* source_c_str;
+    char* source_c_str;
 
     // c-strs containing Sass file contents
     // we will overtake ownership of memory
-    vector<const char*> sources;
+    vector<char*> sources;
+    // strings get freed with context
+    vector<char*> strings;
     // absolute paths to includes
     vector<string> included_files;
     // relative links to includes
@@ -87,7 +89,7 @@ namespace Sass {
     KWD_ARG_SET(Data) {
       KWD_ARG(Data, struct Sass_Options*, c_options)
       KWD_ARG(Data, struct Sass_Compiler*, c_compiler)
-      KWD_ARG(Data, const char*,     source_c_str)
+      KWD_ARG(Data, char*,           source_c_str)
       KWD_ARG(Data, string,          entry_point)
       KWD_ARG(Data, string,          input_path)
       KWD_ARG(Data, string,          output_path)
@@ -117,7 +119,7 @@ namespace Sass {
 
     Block* parse_file();
     Block* parse_string();
-    void add_source(string, string, const char*);
+    void add_source(string, string, char*);
 
     string add_file(const string& file);
     string add_file(const string& base, const string& file);

sass_context.cpp

@@ -606,6 +606,7 @@ extern "C" {
     if (c_ctx == 0) return 0;
     Context::Data cpp_opt = Context::Data();
     cpp_opt.source_c_str(c_ctx->source_string);
+    c_ctx->source_string = 0; // passed away
     return sass_prepare_context(c_ctx, cpp_opt);
   }
 
@@ -620,6 +621,7 @@ extern "C" {
       if (data_ctx->source_string == 0) { throw(runtime_error("Data context has no source string")); }
       if (*data_ctx->source_string == 0) { throw(runtime_error("Data context has empty source string")); }
       cpp_opt.source_c_str(data_ctx->source_string);
+      data_ctx->source_string = 0; // passed away
     }
     catch (...) { return handle_errors(c_ctx) | 1; }
     return sass_compile_context(c_ctx, cpp_opt);
@@ -758,6 +760,7 @@ extern "C" {
     if (ctx->error_file)        free(ctx->error_file);
     if (ctx->input_path)        free(ctx->input_path);
     if (ctx->output_path)       free(ctx->output_path);
+    if (ctx->plugin_path)       free(ctx->plugin_path);
     if (ctx->include_path)      free(ctx->include_path);
     if (ctx->source_map_file)   free(ctx->source_map_file);
     if (ctx->source_map_root)   free(ctx->source_map_root);
@@ -781,16 +784,30 @@ extern "C" {
 
   void ADDCALL sass_delete_compiler (struct Sass_Compiler* compiler)
   {
-    if (compiler == 0) return;
+    if (compiler == 0) {
+      return;
+    }
     Context* cpp_ctx = compiler->cpp_ctx;
     if (cpp_ctx) delete(cpp_ctx);
     compiler->cpp_ctx = 0;
     free(compiler);
   }
 
-  // Deallocate all associated memory with contexts
-  void ADDCALL sass_delete_file_context (struct Sass_File_Context* ctx) { sass_clear_context(ctx); free(ctx); }
-  void ADDCALL sass_delete_data_context (struct Sass_Data_Context* ctx) { sass_clear_context(ctx); free(ctx); }
+  // Deallocate all associated memory with file context
+  void ADDCALL sass_delete_file_context (struct Sass_File_Context* ctx)
+  {
+    // clear the context and free it
+    sass_clear_context(ctx); free(ctx);
+  }
+  // Deallocate all associated memory with data context
+  void ADDCALL sass_delete_data_context (struct Sass_Data_Context* ctx)
+  {
+    // clean the source string if it was not passed
+    // we reset this member once we start parsing
+    if (ctx->source_string) free(ctx->source_string);
+    // clear the context and free it
+    sass_clear_context(ctx); free(ctx);
+  }
 
   // Getters for sass context from specific implementations
   struct Sass_Context* ADDCALL sass_file_context_get_context(struct Sass_File_Context* ctx) { return ctx; }

sass_interface.h

@@ -47,7 +47,7 @@ struct sass_options {
 struct sass_context {
   const char* input_path;
   const char* output_path;
-  const char* source_string;
+  char* source_string;
   char* output_string;
   char* source_map_string;
   struct sass_options options;

source_map.cpp

@@ -19,7 +19,7 @@ namespace Sass {
 
     const bool include_sources = ctx.source_map_contents;
     const vector<string> includes = ctx.include_links;
-    const vector<const char*> sources = ctx.sources;
+    const vector<char*> sources = ctx.sources;
 
     JsonNode* json_srcmap = json_mkobject();