Fix confirmed memory and frontend resource leaks

include/attribute_store.h

@@ -167,7 +167,7 @@ struct AttributePair {
 		else if(hasBoolValue())
 			boost::hash_combine(rv, boolValue());
 		else {
-			throw new std::out_of_range("cannot hash pair, unknown value");
+			throw std::out_of_range("cannot hash pair, unknown value");
 		}
 
 		return rv;

include/helpers.h

@@ -2,7 +2,9 @@
 #ifndef _HELPERS_H
 #define _HELPERS_H
 
+#include <cstdio>
 #include <cstdint>
+#include <memory>
 #include <sstream>
 #include <vector>
 
@@ -41,6 +43,16 @@ struct OffsetAndLength {
 	uint64_t length;
 };
 
+struct FileCloser {
+	void operator()(FILE* fp) const {
+		if (fp != nullptr)
+			fclose(fp);
+	}
+};
+
+using FilePtr = std::unique_ptr<FILE, FileCloser>;
+
+FilePtr openFile(const std::string &filename, const char *mode);
 uint64_t getFileSize(std::string filename);
 std::vector<OffsetAndLength> getNewlineChunks(const std::string &filename, uint64_t chunks);
 

include/shp_processor.h

@@ -38,7 +38,7 @@ class ShpProcessor {
 	void fillPointArrayFromShapefile(std::vector<Point> *points, SHPObject *shape, uint part);
 
 	// Read requested attributes from a shapefile, and encode into an OutputObject
-	AttributeIndex readShapefileAttributes(DBFHandle &dbf, int recordNum, 
+	AttributeIndex readShapefileAttributes(DBFHandle dbf, int recordNum,
 	                                       std::unordered_map<int,std::string> &columnMap,
 	                                       std::unordered_map<int,int> &columnTypeMap,
 	                                       LayerDef &layer, uint &minzoom);
@@ -49,4 +49,3 @@ class ShpProcessor {
 };
 
 #endif //_SHP_PROCESSOR_H
-

src/geojson_processor.cpp

@@ -3,6 +3,7 @@
 #include "helpers.h"
 #include <boost/asio/thread_pool.hpp>
 #include <boost/asio/post.hpp>
+#include <exception>
 
 #include "rapidjson/document.h"
 #include "rapidjson/writer.h"
@@ -24,56 +25,74 @@ void GeoJSONProcessor::read(class LayerDef &layer, uint layerNum) {
 void GeoJSONProcessor::readFeatureCollection(class LayerDef &layer, uint layerNum) {
 	// Read a JSON file containing a single GeoJSON FeatureCollection object.
 	rapidjson::Document doc;
-	FILE* fp = fopen(layer.source.c_str(), "r");
+	FilePtr fp = openFile(layer.source, "r");
 	char readBuffer[65536];
-	rapidjson::FileReadStream is(fp, readBuffer, sizeof(readBuffer));
+	rapidjson::FileReadStream is(fp.get(), readBuffer, sizeof(readBuffer));
 	doc.ParseStream(is);
 	if (doc.HasParseError()) { throw std::runtime_error("Invalid JSON file."); }
-	fclose(fp);
 
 	if (strcmp(doc["type"].GetString(), "FeatureCollection") != 0) { 
 		throw std::runtime_error("Top-level GeoJSON object must be a FeatureCollection.");
 	}
 
 	// Process each feature
+	std::exception_ptr error;
+	std::mutex errorMutex;
 	boost::asio::thread_pool pool(threadNum);
 	for (auto &feature : doc["features"].GetArray()) { 
 		boost::asio::post(pool, [&]() {
-			processFeature(std::move(feature.GetObject()), layer, layerNum);
+			try {
+				processFeature(std::move(feature.GetObject()), layer, layerNum);
+			} catch (...) {
+				std::lock_guard<std::mutex> lock(errorMutex);
+				if (!error)
+					error = std::current_exception();
+			}
 		});
 	}
 	pool.join();
+	if (error)
+		std::rethrow_exception(error);
 }
 
 void GeoJSONProcessor::readFeatureLines(class LayerDef &layer, uint layerNum) {
 	// Read a JSON file containing multiple GeoJSON items, newline-delimited.
 	std::vector<OffsetAndLength> chunks = getNewlineChunks(layer.source, threadNum * 4);
 
 	// Process each feature
+	std::exception_ptr error;
+	std::mutex errorMutex;
 	boost::asio::thread_pool pool(threadNum);
 	for (auto &chunk : chunks) { 
 		boost::asio::post(pool, [&]() {
-			FILE* fp = fopen(layer.source.c_str(), "r");
-			if (fseek(fp, chunk.offset, SEEK_SET) != 0) throw std::runtime_error("unable to seek to " + std::to_string(chunk.offset) + " in " + layer.source);
-			char readBuffer[65536];
-			rapidjson::FileReadStream is(fp, readBuffer, sizeof(readBuffer));
+			try {
+				FilePtr fp = openFile(layer.source, "r");
+				if (fseek(fp.get(), chunk.offset, SEEK_SET) != 0) throw std::runtime_error("unable to seek to " + std::to_string(chunk.offset) + " in " + layer.source);
+				char readBuffer[65536];
+				rapidjson::FileReadStream is(fp.get(), readBuffer, sizeof(readBuffer));
 
-			// Skip leading whitespace.
-			while(is.Tell() < chunk.length && isspace(is.Peek())) is.Take();
+				// Skip leading whitespace.
+				while(is.Tell() < chunk.length && isspace(is.Peek())) is.Take();
 
-			while(is.Tell() < chunk.length) {
-				auto doc = rapidjson::Document();
-				doc.ParseStream<rapidjson::kParseStopWhenDoneFlag>(is);
-				if (doc.HasParseError()) { throw std::runtime_error("Invalid JSON file."); }
-				processFeature(std::move(doc.GetObject()), layer, layerNum);
+				while(is.Tell() < chunk.length) {
+					auto doc = rapidjson::Document();
+					doc.ParseStream<rapidjson::kParseStopWhenDoneFlag>(is);
+					if (doc.HasParseError()) { throw std::runtime_error("Invalid JSON file."); }
+					processFeature(std::move(doc.GetObject()), layer, layerNum);
 
-				// Skip trailing whitespace.
-				while(is.Tell() < chunk.length && isspace(is.Peek())) is.Take();
+					// Skip trailing whitespace.
+					while(is.Tell() < chunk.length && isspace(is.Peek())) is.Take();
+				}
+			} catch (...) {
+				std::lock_guard<std::mutex> lock(errorMutex);
+				if (!error)
+					error = std::current_exception();
 			}
-			fclose(fp);
 		});
 	}
 	pool.join();
+	if (error)
+		std::rethrow_exception(error);
 }
 
 template <bool Flag, typename T>

src/helpers.cpp

@@ -203,6 +203,13 @@ uint64_t getFileSize(std::string filename) {
 	throw std::runtime_error("unable to stat " + filename);
 }
 
+FilePtr openFile(const std::string &filename, const char *mode) {
+	FilePtr fp(fopen(filename.c_str(), mode));
+	if (fp == nullptr)
+		throw std::runtime_error("unable to open " + filename);
+	return fp;
+}
+
 // Given a file, attempt to divide it into N chunks, with each chunk separated
 // by a newline.
 //
@@ -212,7 +219,7 @@ std::vector<OffsetAndLength> getNewlineChunks(const std::string &filename, uint6
 
 	const uint64_t size = getFileSize(filename);
 	const uint64_t chunkSize = std::max<uint64_t>(size / chunks, 1ul);
-	FILE* fp = fopen(filename.c_str(), "r");
+	FilePtr fp = openFile(filename, "r");
 
 	// Our approach is naive: skip chunkSize bytes, scan for a newline, repeat.
 	//
@@ -226,12 +233,12 @@ std::vector<OffsetAndLength> getNewlineChunks(const std::string &filename, uint6
 		// The last chunk will not be a full `chunkSize`.
 		length = std::min(chunkSize, size - offset);
 
-		if (fseek(fp, offset + length, SEEK_SET) != 0) throw std::runtime_error("unable to seek to " + std::to_string(offset) + " in " + filename);
+		if (fseek(fp.get(), offset + length, SEEK_SET) != 0) throw std::runtime_error("unable to seek to " + std::to_string(offset) + " in " + filename);
 
 		bool foundNewline = false;
 
 		while(!foundNewline) {
-			size_t read = fread(buffer, 1, sizeof(buffer), fp);
+			size_t read = fread(buffer, 1, sizeof(buffer), fp.get());
 			if (read == 0) break;
 			for (int i = 0; i < read; i++) {
 				if (buffer[i] == '\n') {
@@ -248,6 +255,5 @@ std::vector<OffsetAndLength> getNewlineChunks(const std::string &filename, uint6
 		offset += length;
 	}
 
-	fclose(fp);
 	return rv;
 }

src/pooled_string.cpp

@@ -2,10 +2,18 @@
 #include <stdexcept>
 #include <mutex>
 #include <cstring>
+#include <cstdlib>
 #include <deque>
+#include <memory>
 
 namespace PooledStringNS {
-	std::deque<char*> tables;
+	struct TableDeleter {
+		void operator()(char* table) const {
+			std::free(table);
+		}
+	};
+
+	std::deque<std::unique_ptr<char, TableDeleter>> tables;
 	std::mutex mutex;
 
 	const uint8_t ShortString = 0b00;
@@ -33,10 +41,10 @@ PooledString::PooledString(const std::string& str) {
 		if (spaceLeft < 0 || spaceLeft < str.size()) {
 			std::lock_guard<std::mutex> lock(mutex);
 			spaceLeft = 65536;
-			char* buffer = (char*)malloc(spaceLeft);
-			if (buffer == 0)
+			std::unique_ptr<char, TableDeleter> buffer(static_cast<char*>(std::malloc(spaceLeft)));
+			if (!buffer)
 				throw std::runtime_error("PooledString could not malloc");
-			tables.push_back(buffer);
+			tables.emplace_back(std::move(buffer));
 			tableIndex = tables.size() - 1;
 		}
 
@@ -52,7 +60,7 @@ PooledString::PooledString(const std::string& str) {
 		storage[6] = length >> 8;
 		storage[7] = length;
 
-		memcpy(tables[tableIndex] + offset, str.data(), str.size());
+		memcpy(tables[tableIndex].get() + offset, str.data(), str.size());
 
 		spaceLeft -= str.size();
 	}
@@ -106,7 +114,7 @@ const char* PooledStringNS::PooledString::data() const {
 	uint32_t tableIndex = (storage[1] << 16) + (storage[2] << 8) + storage[3];
 	uint16_t offset = (storage[4] << 8) + storage[5];
 
-	const char* data = tables[tableIndex] + offset;
+	const char* data = tables[tableIndex].get() + offset;
 	return data;
 }
 
@@ -136,7 +144,7 @@ std::string PooledStringNS::PooledString::toString() const {
 		uint32_t tableIndex = (storage[1] << 16) + (storage[2] << 8) + storage[3];
 		uint16_t offset = (storage[4] << 8) + storage[5];
 
-		char* data = tables[tableIndex] + offset;
+		char* data = tables[tableIndex].get() + offset;
 		rv.append(data, size());
 		return rv;
 	}
@@ -169,4 +177,3 @@ bool PooledStringNS::PooledString::operator<(const PooledString& other) const {
 
 	return memcmp(data(), other.data(), mySize) < 0;
 }
-