From f80257d6c19a03acbfdee51cae8ccbf21866bd4b Mon Sep 17 00:00:00 2001
From: Otavio Pontes <otavio.pontes@intel.com>
Date: Wed, 27 Jun 2018 22:35:11 +0000
Subject: [PATCH] download: remove mutex lock in single thread code

Functions clean_curl_multi_queue() and swupd_curl_hashmap_insert() are
always called in the same thread so there isn't any need to protect
access to swupd_curl_hashbucket or swupd_curl_hashmap.

The download.c functions use curl_multi infrastructure to download in parallel
but all processing is performed in the main thread. So we don't need to worry
about concurrency problems in object access.

But we need to worry about concurrency in file access. Fortunatelly the hashmap
prevents the same file to be downloaded twice, so we are safe on that.
---
 Makefile.am    |  3 +--
 configure.ac   |  1 -
 src/download.c | 35 ++++++++++-------------------------
 3 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 9a6abff4..d5020899 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -63,8 +63,7 @@ swupd_LDADD = \
 	$(openssl_LIBS) \
 	$(curl_LIBS) \
 	$(bsdiff_LIBS) \
-	$(libarchive_LIBS) \
-	$(pthread_LIBS)
+	$(libarchive_LIBS)
 
 verifytime_SOURCES = src/verifytime.c
 bin_PROGRAMS += verifytime
diff --git a/configure.ac b/configure.ac
index b5e0b41d..8e5153ac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -22,7 +22,6 @@ PKG_CHECK_MODULES([zlib], [zlib])
 PKG_CHECK_MODULES([curl], [libcurl])
 PKG_CHECK_MODULES([openssl], [libcrypto >= 1.0.1])
 PKG_CHECK_MODULES([libarchive], [libarchive])
-AC_CHECK_LIB([pthread], [pthread_create])
 
 
 # Program checks
diff --git a/src/download.c b/src/download.c
index 5796bc17..2ed0195e 100644
--- a/src/download.c
+++ b/src/download.c
@@ -26,7 +26,6 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/fs.h>
-#include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -61,12 +60,8 @@ static struct list *failed = NULL;
  *
  * file->hash[0] acts as index into the arrays
  */
-struct swupd_curl_hashbucket {
-	pthread_mutex_t mutex;
-	struct list *list;
-};
 #define SWUPD_CURL_HASH_BUCKETS 256
-static struct swupd_curl_hashbucket swupd_curl_hashmap[SWUPD_CURL_HASH_BUCKETS];
+static struct list *swupd_curl_hashmap[SWUPD_CURL_HASH_BUCKETS];
 
 /* try to insert the file into the hashmap download queue
  * returns 1 if no download is needed
@@ -80,16 +75,15 @@ static int swupd_curl_hashmap_insert(struct file *file)
 	char *targetfile;
 	struct stat stat;
 	int hashmap_index = file->hash[0];
-	struct swupd_curl_hashbucket *bucket = &swupd_curl_hashmap[hashmap_index];
+	struct list **bucket;
 
-	pthread_mutex_lock(&bucket->mutex);
+	bucket = &swupd_curl_hashmap[hashmap_index];
 
-	iter = bucket->list;
+	iter = *bucket;
 	while (iter) {
 		tmp = iter->data;
 		if (hash_equal(tmp->hash, file->hash)) {
 			// hash already in download queue
-			pthread_mutex_unlock(&bucket->mutex);
 			return 1;
 		}
 		iter = iter->next;
@@ -103,7 +97,6 @@ static int swupd_curl_hashmap_insert(struct file *file)
 		if (verify_file(file, targetfile)) {
 			/* hash matches, no download necessary */
 			free_string(&targetfile);
-			pthread_mutex_unlock(&bucket->mutex);
 			return 1;
 		} else {
 			/* hash mismatch, remove the staged file to enable re-download */
@@ -121,14 +114,12 @@ static int swupd_curl_hashmap_insert(struct file *file)
 	free_string(&tar_dotfile);
 
 	// queue the hash for download
-	iter = bucket->list;
+	iter = *bucket;
 	if ((iter = list_prepend_data(iter, file)) == NULL) {
-		pthread_mutex_unlock(&bucket->mutex);
 		return -1;
 	}
-	bucket->list = iter;
+	*bucket = iter;
 
-	pthread_mutex_unlock(&bucket->mutex);
 	return 0;
 }
 
@@ -138,12 +129,7 @@ static size_t MAX_XFER_BOTTOM = 15;
 
 int start_full_download(bool pipelining)
 {
-	int i;
-
 	failed = NULL;
-	for (i = 0; i < SWUPD_CURL_HASH_BUCKETS; i++) {
-		pthread_mutex_init(&swupd_curl_hashmap[i].mutex, NULL);
-	}
 
 	mcurl = curl_multi_init();
 	if (mcurl == NULL) {
@@ -183,14 +169,13 @@ static void free_curl_list_data(void *data)
 void clean_curl_multi_queue(void)
 {
 	int i;
-	struct swupd_curl_hashbucket *bucket;
+	struct list **bucket;
 
 	for (i = 0; i < SWUPD_CURL_HASH_BUCKETS; i++) {
 		bucket = &swupd_curl_hashmap[i];
-		pthread_mutex_lock(&bucket->mutex);
-		list_free_list_and_data(bucket->list, free_curl_list_data);
-		bucket->list = NULL;
-		pthread_mutex_unlock(&bucket->mutex);
+
+		list_free_list_and_data(*bucket, free_curl_list_data);
+		*bucket = NULL;
 	}
 }