From f4e11e75fe72e9257f7086966a6a480e5f3684a6 Mon Sep 17 00:00:00 2001
From: Simon Rettberg
Date: Wed, 28 Aug 2019 10:34:22 +0200
Subject: [SERVER] Handle closeUnusedFd via timer

---
 src/server/altservers.c |  8 --------
 src/server/image.c      | 36 +++++++++++++++++++-----------------
 2 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/src/server/altservers.c b/src/server/altservers.c
index 7d7fdbe..e088601 100644
--- a/src/server/altservers.c
+++ b/src/server/altservers.c
@@ -20,7 +20,6 @@
 static dnbd3_alt_server_t altServers[SERVER_MAX_ALTS];
 static atomic_int numAltServers = 0;
 static pthread_mutex_t altServersLock;
-static ticks nextCloseUnusedFd; // TODO: Move away
 
 static void *altservers_runCheck(void *data);
 static int altservers_getListForUplink(dnbd3_uplink_t *uplink, int *servers, int size, int current);
@@ -380,13 +379,6 @@ static void *altservers_runCheck(void *data)
 	setThreadName( "altserver-check" );
 	altservers_findUplinkInternal( uplink );
 	ref_put( &uplink->reference ); // Acquired in findUplinkAsync
-	// Save cache maps of all images if applicable
-	// TODO: Has nothing to do with alt servers really, maybe move somewhere else?
-	declare_now;
-	if ( _closeUnusedFd && timing_reached( &nextCloseUnusedFd, &now ) ) {
-		timing_gets( &nextCloseUnusedFd, 900 );
-		image_closeUnusedFd();
-	}
 	return NULL;
 }
 
diff --git a/src/server/image.c b/src/server/image.c
index 5b58347..ace585b 100644
--- a/src/server/image.c
+++ b/src/server/image.c
@@ -54,6 +54,7 @@ static bool image_ensureDiskSpace(uint64_t size, bool force);
 static uint8_t* image_loadCacheMap(const char * const imagePath, const int64_t fileSize);
 static uint32_t* image_loadCrcList(const char * const imagePath, const int64_t fileSize, uint32_t *masterCrc);
 static bool image_checkRandomBlocks(const int count, int fdImage, const int64_t fileSize, uint32_t * const crc32list, uint8_t * const cache_map);
+static void* closeUnusedFds(void*);
 
 // ##########################################
 
@@ -63,6 +64,7 @@ void image_serverStartup()
 	mutex_init( &imageListLock, LOCK_IMAGE_LIST );
 	mutex_init( &remoteCloneLock, LOCK_REMOTE_CLONE );
 	mutex_init( &reloadLock, LOCK_RELOAD );
+	server_addJob( &closeUnusedFds, NULL, 10, 900 );
 }
 
 /**
@@ -1717,34 +1719,34 @@ static bool image_ensureDiskSpace(uint64_t size, bool force)
 	return false;
 }
 
-void image_closeUnusedFd()
+#define FDCOUNT (400)
+static void* closeUnusedFds(void* nix UNUSED)
 {
-	int fd, i;
+	if ( !_closeUnusedFd )
+		return NULL;
 	ticks deadline;
 	timing_gets( &deadline, -UNUSED_FD_TIMEOUT );
-	char imgstr[300];
+	int fds[FDCOUNT];
+	int fdindex = 0;
 	mutex_lock( &imageListLock );
-	for (i = 0; i < _num_images; ++i) {
+	for ( int i = 0; i < _num_images; ++i ) {
 		dnbd3_image_t * const image = _images[i];
 		if ( image == NULL )
 			continue;
-		mutex_lock( &image->lock );
 		if ( image->users == 0 && image->uplinkref == NULL && timing_reached( &image->atime, &deadline ) ) {
-			snprintf( imgstr, sizeof(imgstr), "%s:%d", image->name, (int)image->rid );
-			fd = image->readFd;
-			image->readFd = -1;
-		} else {
-			fd = -1;
-		}
-		mutex_unlock( &image->lock );
-		if ( fd != -1 ) {
-			mutex_unlock( &imageListLock );
-			close( fd );
-			logadd( LOG_DEBUG1, "Inactive fd closed for %s", imgstr );
-			mutex_lock( &imageListLock );
+			logadd( LOG_DEBUG1, "Inactive fd closed for %s:%d", image->name, (int)image->rid );
+			fds[fdindex++] = image->readFd;
+			image->readFd = -1; // Not a race; image->users is 0 and to increase it you need imageListLock
+			if ( fdindex == FDCOUNT )
+				break;
 		}
 	}
 	mutex_unlock( &imageListLock );
+	// Do this after unlock since close might block
+	for ( int i = 0; i < fdindex; ++i ) {
+		close( fds[i] );
+	}
+	return NULL;
 }
 
 /*
-- 
cgit v1.2.3-55-g7522