fix memleak / improve curlwrapper

Author: abma

src/Downloader/CurlWrapper.cpp

@@ -1,30 +1,32 @@
-
+/* This file is part of pr-downloader (GPL v2 or later), see the LICENSE file */
 
 #include <curl/curl.h>
 
 #include "CurlWrapper.h"
 #include "Version.h"
 
-CURL* CurlWrapper::CurlInit()
+CurlWrapper::CurlWrapper()
 {
-	CURL* ret;
-	ret = curl_easy_init();
-	curl_easy_setopt(ret, CURLOPT_CONNECTTIMEOUT, 30);
+	handle = curl_easy_init();
+	curl_easy_setopt(handle, CURLOPT_CONNECTTIMEOUT, 30);
 
 	//if transfer is slower this bytes/s than this for CURLOPT_LOW_SPEED_TIME then its aborted
-	curl_easy_setopt(ret, CURLOPT_LOW_SPEED_LIMIT, 10);
-	curl_easy_setopt(ret, CURLOPT_LOW_SPEED_TIME, 30);
-	curl_easy_setopt(ret, CURLOPT_PROTOCOLS, CURLPROTO_HTTP|CURLPROTO_HTTPS);
-	curl_easy_setopt(ret, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTP|CURLPROTO_HTTPS);
-	curl_easy_setopt(ret, CURLOPT_USERAGENT, getVersion());
-	curl_easy_setopt(ret, CURLOPT_FAILONERROR, true);
-	curl_easy_setopt(ret, CURLOPT_FOLLOWLOCATION, 1);
+	curl_easy_setopt(handle, CURLOPT_LOW_SPEED_LIMIT, 10);
+	curl_easy_setopt(handle, CURLOPT_LOW_SPEED_TIME, 30);
+	curl_easy_setopt(handle, CURLOPT_PROTOCOLS, CURLPROTO_HTTP|CURLPROTO_HTTPS);
+	curl_easy_setopt(handle, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTP|CURLPROTO_HTTPS);
+	curl_easy_setopt(handle, CURLOPT_USERAGENT, getVersion());
+	curl_easy_setopt(handle, CURLOPT_FAILONERROR, true);
+	curl_easy_setopt(handle, CURLOPT_FOLLOWLOCATION, 1);
 
-	struct curl_slist *list = NULL;
-        list = curl_slist_append(list, "Cache-Control: no-cache");
-        curl_easy_setopt(ret, CURLOPT_HTTPHEADER, list);
-//FIXME: memleak, has to be freed at shutdown
-//        curl_slist_free_all(list); /* free the list again */
+	list = NULL;
+	list = curl_slist_append(list, "Cache-Control: no-cache");
+	curl_easy_setopt(handle, CURLOPT_HTTPHEADER, list);
+}
 
-	return ret;
+CurlWrapper::~CurlWrapper()
+{
+	curl_easy_cleanup(handle);
+	handle = NULL;
+	curl_slist_free_all(list); /* free the list again */
 }

src/Downloader/CurlWrapper.h

@@ -1,7 +1,16 @@
+/* This file is part of pr-downloader (GPL v2 or later), see the LICENSE file */
 
 #include <curl/curl.h>
 
-class CurlWrapper {
+class CurlWrapper
+{
 public:
-	static CURL* CurlInit();
+	CurlWrapper();
+	~CurlWrapper();
+	CURL* const GetHandle() const {
+		return handle;
+	}
+private:
+	CURL* handle;
+	struct curl_slist *list;
 };

src/Downloader/Http/DownloadData.cpp

@@ -10,15 +10,13 @@ DownloadData::DownloadData()
 	start_piece=0;
 	mirror=NULL;
 	download=NULL;
-	easy_handle=CurlWrapper::CurlInit();
+	curlw = new CurlWrapper();
 	got_ranges = false;
 }
 
 DownloadData::~DownloadData()
 {
-	if (easy_handle!=NULL) {
-		curl_easy_cleanup(easy_handle);
-		easy_handle=NULL;
-	}
+	delete curlw;
+	curlw = NULL;
 }
 

src/Downloader/Http/DownloadData.h

@@ -3,12 +3,11 @@
 #ifndef _DOWNLOAD_DATA_H
 #define _DOWNLOAD_DATA_H
 
-#include <string>
 #include <vector>
 class CFile;
 class Mirror;
 class IDownload;
-typedef void CURL;
+class CurlWrapper;
 
 class DownloadData
 {
@@ -17,7 +16,7 @@ class DownloadData
 	~DownloadData();
 	int start_piece;
 	std::vector<unsigned int> pieces;
-	CURL* easy_handle; //curl_easy_handle
+	CurlWrapper* curlw; //curl_easy_handle
 	Mirror* mirror; //mirror used
 	IDownload *download;
 	bool got_ranges; //true if headers received from server are fine

src/Downloader/Http/HttpDownloader.cpp

@@ -26,11 +26,11 @@
 #include <sstream>
 #include <stdlib.h>
 
-class xmllog: public XmlRpc::XmlRpcLogHandler {
+class xmllog: public XmlRpc::XmlRpcLogHandler
+{
 public:
 	virtual ~xmllog() {}
-	void log(int level, const char* msg)
-	{
+	void log(int level, const char* msg) {
 		LOG_INFO("%s",msg);
 	}
 };
@@ -61,12 +61,12 @@ CHttpDownloader::~CHttpDownloader()
 
 bool CHttpDownloader::search(std::list<IDownload*>& res, const std::string& name, IDownload::category cat)
 {
-	CURL* curl = CurlWrapper::CurlInit();
+	CurlWrapper* curlw = new CurlWrapper();
 	LOG_DEBUG("%s", name.c_str()  );
 
 	const std::string method(XMLRPC_METHOD);
 	//std::string category;
-	XmlRpc::XmlRpcCurlClient client(curl, XMLRPC_HOST,XMLRPC_PORT, XMLRPC_URI);
+	XmlRpc::XmlRpcCurlClient client(curlw->GetHandle(), XMLRPC_HOST,XMLRPC_PORT, XMLRPC_URI);
 	XmlRpc::XmlRpcValue arg;
 	arg["springname"]=name;
 	arg["torrent"]=true;
@@ -99,17 +99,20 @@ bool CHttpDownloader::search(std::list<IDownload*>& res, const std::string& name
 
 	if (result.getType()!=XmlRpc::XmlRpcValue::TypeArray) {
 		LOG_ERROR("Returned xml isn't an array!");
+		delete curlw;
 		return false;
 	}
 
 	for(int i=0; i<result.size(); i++) {
 		XmlRpc::XmlRpcValue resfile = result[i];
 
 		if (resfile.getType()!=XmlRpc::XmlRpcValue::TypeStruct) {
+			delete curlw;
 			return false;
 		}
 		if (resfile["category"].getType()!=XmlRpc::XmlRpcValue::TypeString) {
 			LOG_ERROR("No category in result");
+			delete curlw;
 			return false;
 		}
 		std::string filename=fileSystem->getSpringDir();
@@ -125,8 +128,9 @@ bool CHttpDownloader::search(std::list<IDownload*>& res, const std::string& name
 			LOG_ERROR("Unknown Category %s", category.c_str());
 		filename+=PATH_DELIMITER;
 		if ((resfile["mirrors"].getType()!=XmlRpc::XmlRpcValue::TypeArray) ||
-			(resfile["filename"].getType()!=XmlRpc::XmlRpcValue::TypeString)) {
+		    (resfile["filename"].getType()!=XmlRpc::XmlRpcValue::TypeString)) {
 			LOG_ERROR("Invalid type in result");
+			delete curlw;
 			return false;
 		}
 		filename.append(resfile["filename"]);
@@ -165,6 +169,7 @@ bool CHttpDownloader::search(std::list<IDownload*>& res, const std::string& name
 		}
 		res.push_back(dl);
 	}
+	delete curlw;
 	return true;
 }
 
@@ -299,14 +304,14 @@ bool CHttpDownloader::setupDownload(DownloadData* piece)
 	piece->start_piece=pieces.size() > 0 ? pieces[0] : -1;
 	assert(piece->download->pieces.size()<=0 || piece->start_piece >=0);
 	piece->pieces = pieces;
-	if (piece->easy_handle==NULL) {
-		piece->easy_handle=CurlWrapper::CurlInit();
+	if (piece->curlw==NULL) {
+		piece->curlw = new CurlWrapper();
 	} else {
-		curl_easy_cleanup(piece->easy_handle);
-		piece->easy_handle=CurlWrapper::CurlInit();
+		delete piece->curlw;
+		piece->curlw = new CurlWrapper();
 	}
 
-	CURL* curle= piece->easy_handle;
+	CURL* curle = piece->curlw->GetHandle();
 	piece->mirror=piece->download->getFastestMirror();
 	if (piece->mirror==NULL) {
 		LOG_ERROR("No mirror found");
@@ -351,7 +356,7 @@ bool CHttpDownloader::setupDownload(DownloadData* piece)
 DownloadData* CHttpDownloader::getDataByHandle(const std::vector <DownloadData*>& downloads, const CURL* easy_handle) const
 {
 	for(int i=0; i<(int)downloads.size(); i++) { //search corresponding data structure
-		if (downloads[i]->easy_handle == easy_handle) {
+		if (downloads[i]->curlw->GetHandle() == easy_handle) {
 			return downloads[i];
 		}
 	}
@@ -411,22 +416,22 @@ bool CHttpDownloader::processMessages(CURLM* curlm, std::vector <DownloadData*>&
 			}
 			//get speed at which this piece was downloaded + update mirror info
 			double dlSpeed;
-			curl_easy_getinfo(data->easy_handle, CURLINFO_SPEED_DOWNLOAD, &dlSpeed);
+			curl_easy_getinfo(data->curlw->GetHandle(), CURLINFO_SPEED_DOWNLOAD, &dlSpeed);
 			data->mirror->UpdateSpeed(dlSpeed);
 			if (data->mirror->status == Mirror::STATUS_UNKNOWN) //set mirror status only when unset
 				data->mirror->status=Mirror::STATUS_OK;
 
 			//remove easy handle, as its finished
-			curl_multi_remove_handle(curlm, data->easy_handle);
-			curl_easy_cleanup(data->easy_handle);
-			data->easy_handle=NULL;
+			curl_multi_remove_handle(curlm, data->curlw->GetHandle());
+			delete data->curlw;
+			data->curlw=NULL;
 			LOG_INFO("piece finished");
 			//piece finished / failed, try a new one
 			if (!setupDownload(data)) {
 				LOG_DEBUG("No piece found, all pieces finished / currently downloading");
 				break;
 			}
-			int ret=curl_multi_add_handle(curlm, data->easy_handle);
+			int ret=curl_multi_add_handle(curlm, data->curlw->GetHandle());
 			if (ret!=CURLM_OK) {
 				LOG_ERROR("curl_multi_perform_error: %d %d", ret, CURLM_BAD_EASY_HANDLE);
 			}
@@ -474,7 +479,7 @@ bool CHttpDownloader::download(std::list<IDownload*>& download, int max_parallel
 				}
 			} else {
 				downloads.push_back(dlData);
-				curl_multi_add_handle(curlm, dlData->easy_handle);
+				curl_multi_add_handle(curlm, dlData->curlw->GetHandle());
 			}
 		}
 	}
@@ -516,7 +521,7 @@ bool CHttpDownloader::download(std::list<IDownload*>& download, int max_parallel
 		double size=-1;
 		for (unsigned i=0; i<downloads.size(); i++) {
 			double tmp;
-			curl_easy_getinfo(downloads[i]->easy_handle, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &tmp);
+			curl_easy_getinfo(downloads[i]->curlw->GetHandle(), CURLINFO_CONTENT_LENGTH_DOWNLOAD, &tmp);
 			if (tmp>size) {
 				size=tmp;
 			}
@@ -541,7 +546,7 @@ bool CHttpDownloader::download(std::list<IDownload*>& download, int max_parallel
 	}
 	for (unsigned i=0; i<downloads.size(); i++) {
 		long timestamp;
-		if (curl_easy_getinfo(downloads[i]->easy_handle, CURLINFO_FILETIME, &timestamp) == CURLE_OK) {
+		if (curl_easy_getinfo(downloads[i]->curlw->GetHandle(), CURLINFO_FILETIME, &timestamp) == CURLE_OK) {
 			if (downloads[i]->download->state != IDownload::STATE_FINISHED) //decrease local timestamp if download failed to force redownload next time
 				timestamp--;
 			downloads[i]->download->file->SetTimestamp(timestamp);

src/Downloader/Rapid/Sdp.cpp

@@ -244,52 +244,54 @@ static int progress_func(CSdp& csdp, double TotalToDownload, double NowDownloade
 
 bool CSdp::downloadStream(const std::string& url,std::list<FileData*> files)
 {
-	CURL* curl = CurlWrapper::CurlInit();
-	if (curl) {
-		CURLcode res;
-		LOG_INFO("Using rapid");
-		LOG_INFO(url.c_str());
+	CurlWrapper* curlw = new CurlWrapper();
+	if (!curlw) {
+		return false;
+	}
+	CURLcode res;
+	LOG_INFO("Using rapid");
+	LOG_INFO(url.c_str());
 
-		curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
+	curl_easy_setopt(curlw->GetHandle(), CURLOPT_URL, url.c_str());
 
-		int  buflen=files.size()/8;
-		if (files.size()%8!=0)
-			buflen++;
-		char* buf=(char*)malloc(buflen); //FIXME: compress blockwise and not all at once
-		memset(buf,0,buflen);
-		int destlen=files.size()*2;
-		LOG_DEBUG("%d %d %d",(int)files.size(),buflen,destlen);
-		int i=0;
-		std::list<FileData*>::iterator it;
-		for (it=files.begin(); it!=files.end(); ++it) {
-			if ((*it)->download==true) {
-				buf[i/8] |= (1<<(i%8));
-			}
-			i++;
+	int  buflen=files.size()/8;
+	if (files.size()%8!=0)
+		buflen++;
+	char* buf=(char*)malloc(buflen); //FIXME: compress blockwise and not all at once
+	memset(buf,0,buflen);
+	int destlen=files.size()*2;
+	LOG_DEBUG("%d %d %d",(int)files.size(),buflen,destlen);
+	int i=0;
+	std::list<FileData*>::iterator it;
+	for (it=files.begin(); it!=files.end(); ++it) {
+		if ((*it)->download==true) {
+			buf[i/8] |= (1<<(i%8));
 		}
-		char* dest=(char*)malloc(destlen);
+		i++;
+	}
+	char* dest=(char*)malloc(destlen);
 
-		gzip_str(buf,buflen,dest,&destlen);
+	gzip_str(buf,buflen,dest,&destlen);
 
-		curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_streamed_data);
-		curl_easy_setopt(curl, CURLOPT_WRITEDATA, this);
+	curl_easy_setopt(curlw->GetHandle(), CURLOPT_WRITEFUNCTION, write_streamed_data);
+	curl_easy_setopt(curlw->GetHandle(), CURLOPT_WRITEDATA, this);
 
 
-		globalFiles=&files;
-		curl_easy_setopt(curl, CURLOPT_POSTFIELDS, dest);
-		curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE,destlen);
-		curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L);
-		curl_easy_setopt(curl, CURLOPT_PROGRESSFUNCTION, progress_func);
-		curl_easy_setopt(curl, CURLOPT_PROGRESSDATA, this);
+	globalFiles=&files;
+	curl_easy_setopt(curlw->GetHandle(), CURLOPT_POSTFIELDS, dest);
+	curl_easy_setopt(curlw->GetHandle(), CURLOPT_POSTFIELDSIZE,destlen);
+	curl_easy_setopt(curlw->GetHandle(), CURLOPT_NOPROGRESS, 0L);
+	curl_easy_setopt(curlw->GetHandle(), CURLOPT_PROGRESSFUNCTION, progress_func);
+	curl_easy_setopt(curlw->GetHandle(), CURLOPT_PROGRESSDATA, this);
 
-		res = curl_easy_perform(curl);
-		free(dest);
-		/* always cleanup */
-		curl_easy_cleanup(curl);
-		if (res!=CURLE_OK) {
-			LOG_ERROR("Curl cleanup error: %s",curl_easy_strerror(res));
-			return false;
-		}
+	res = curl_easy_perform(curlw->GetHandle());
+	free(dest);
+	/* always cleanup */
+	delete curlw;
+	curlw = NULL;
+	if (res != CURLE_OK) {
+		LOG_ERROR("Curl cleanup error: %s",curl_easy_strerror(res));
+		return false;
 	}
 	return true;
 }