Skip to content

Commit

Permalink
Fix memory leaks and improve input_source handling
Browse files Browse the repository at this point in the history
  • Loading branch information
mgreter committed Jul 7, 2015
1 parent 5776e9b commit af12e9b
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 20 deletions.
27 changes: 17 additions & 10 deletions context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>()),
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 6 additions & 4 deletions context.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
25 changes: 21 additions & 4 deletions sass_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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; }
Expand Down
2 changes: 1 addition & 1 deletion sass_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion source_map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down

0 comments on commit af12e9b

Please sign in to comment.