[SERVER] Fix automatic proxying to use supplied connection; fix race condition in uplink_init

6 files changed, 45 insertions, 37 deletions

diff --git a/src/config.h b/src/config.h
index 6f8c33e..271e4bc 100644
--- a/src/config.h
+++ b/src/config.h
@@ -41,7 +41,7 @@
 #define SERVER_RTT_DELAY_MAX 45
 #define SERVER_RTT_DELAY_FAILED 180
 
-#define SERVER_REMOTE_IMAGE_CHECK_CACHETIME 600 // 10 minutes
+#define SERVER_REMOTE_IMAGE_CHECK_CACHETIME 120 // 2 minutes
 #define SERVER_MAX_PROXY_IMAGE_SIZE 100000000000LL // 100GB
 // +++++ Network +++++
 // Default port
diff --git a/src/server/globals.h b/src/server/globals.h
index 1edf44f..2ea1c43 100644
--- a/src/server/globals.h
+++ b/src/server/globals.h
@@ -107,7 +107,7 @@ struct _dnbd3_image
 	int rid;               // revision of image
 	int users;             // clients currently using this image
 	time_t atime;          // last access time
-	bool working;          // true if image exists and completeness is == 100% or a working upstream proxy is connected
+	volatile bool working; // true if image exists and completeness is == 100% or a working upstream proxy is connected
 	pthread_spinlock_t lock;
 };
 
diff --git a/src/server/image.c b/src/server/image.c
index 27b737e..d8c7113 100644
--- a/src/server/image.c
+++ b/src/server/image.c
@@ -40,7 +40,7 @@ typedef struct
 	time_t deadline;
 } imagecache;
 static imagecache remoteCloneCache[CACHELEN];
-static int remoteCloneCacheIndex = -1;
+static int remoteCloneCacheIndex = 0;
 
 // ##########################################
 
@@ -232,7 +232,7 @@ bool image_saveCacheMap(dnbd3_image_t *image)
  * point...
  * Locks on: _images_lock, _images[].lock
  */
-dnbd3_image_t* image_get(char *name, uint16_t revision)
+dnbd3_image_t* image_get(char *name, uint16_t revision, bool checkIfWorking)
 {
 	int i;
 	dnbd3_image_t *candidate = NULL;
@@ -265,6 +265,8 @@ dnbd3_image_t* image_get(char *name, uint16_t revision)
 	candidate->users++;
 	spin_unlock( &candidate->lock );
 
+	if ( !checkIfWorking ) return candidate;
+
 	// Found, see if it works
 	struct stat st;
 	if ( candidate->working && stat( candidate->path, &st ) < 0 ) {
@@ -420,16 +422,18 @@ static dnbd3_image_t* image_free(dnbd3_image_t *image)
 	//
 	image_saveCacheMap( image );
 	uplink_shutdown( image );
+	spin_lock( &image->lock );
 	free( image->cache_map );
 	free( image->crc32 );
 	free( image->path );
 	free( image->lower_name );
+	spin_unlock( &image->lock );
 	if ( image->cacheFd != -1 ) {
 		close( image->cacheFd );
 	}
 	spin_destroy( &image->lock );
 	//
-	memset( image, 0, sizeof(dnbd3_image_t) );
+	memset( image, 0, sizeof(*image) );
 	free( image );
 	return NULL ;
 }
@@ -557,7 +561,7 @@ static bool image_load(char *base, char *path, int withUplink)
 	strtolower( imgName );
 
 	// Get pointer to already existing image if possible
-	existing = image_get( imgName, revision );
+	existing = image_get( imgName, revision, true );
 
 	// ### Now load the actual image related data ###
 	fdImage = open( path, O_RDONLY );
@@ -874,19 +878,15 @@ bool image_create(char *image, int revision, uint64_t size)
  */
 dnbd3_image_t* image_getOrClone(char *name, uint16_t revision)
 {
-	if ( !_isProxy ) return image_get( name, revision );
+	if ( !_isProxy ) return image_get( name, revision, true );
 	int i;
 	const size_t len = strlen( name );
 	// Sanity check
 	if ( len == 0 || name[len - 1] == '/' || name[0] == '/' ) return NULL ;
 	// Already existing locally?
-	dnbd3_image_t *image = image_get( name, revision );
+	dnbd3_image_t *image = image_get( name, revision, true );
 	if ( image != NULL ) return image;
 	// Doesn't exist, try remote if not already tried it recently
-	if ( remoteCloneCacheIndex == -1 ) {
-		remoteCloneCacheIndex = 0;
-		memset( remoteCloneCache, 0, sizeof(remoteCloneCache) );
-	}
 	const time_t now = time( NULL );
 
 	char *cmpname = name;
@@ -897,10 +897,10 @@ dnbd3_image_t* image_getOrClone(char *name, uint16_t revision)
 		        || remoteCloneCache[i].deadline < now
 		        || strcmp( cmpname, remoteCloneCache[i].name ) != 0 ) continue;
 		pthread_mutex_unlock( &remoteCloneLock ); // Was recently checked...
-		return image_get( name, revision );
+		return image_get( name, revision, true );
 	}
 	// Re-check to prevent two clients at the same time triggering this
-	image = image_get( name, revision );
+	image = image_get( name, revision, true );
 	if ( image != NULL ) {
 		pthread_mutex_unlock( &remoteCloneLock );
 		return image;
@@ -940,11 +940,13 @@ dnbd3_image_t* image_getOrClone(char *name, uint16_t revision)
 	}
 	pthread_mutex_unlock( &remoteCloneLock );
 	// If everything worked out, this call should now actually return the image
-	image = image_get( name, remoteRid );
+	image = image_get( name, remoteRid, false );
 	if ( image != NULL && uplinkSock != -1 && uplinkServer != NULL ) {
 		// If so, init the uplink and pass it the socket
 		uplink_init( image, uplinkSock, uplinkServer );
-		image->working = true;
+		i = 0;
+		while ( !image->working && ++i < 100 )
+			usleep( 1000 );
 	} else if ( uplinkSock >= 0 ) {
 		close( uplinkSock );
 	}
diff --git a/src/server/image.h b/src/server/image.h
index 664225c..aca3788 100644
--- a/src/server/image.h
+++ b/src/server/image.h
@@ -18,7 +18,7 @@ void image_saveAllCacheMaps();
 
 bool image_saveCacheMap(dnbd3_image_t *image);
 
-dnbd3_image_t* image_get(char *name, uint16_t revision);
+dnbd3_image_t* image_get(char *name, uint16_t revision, bool checkIfWorking);
 
 dnbd3_image_t* image_getOrClone(char *name, uint16_t revision);
 
diff --git a/src/server/threadpool.c b/src/server/threadpool.c
index fd90944..2d621c5 100644
--- a/src/server/threadpool.c
+++ b/src/server/threadpool.c
@@ -72,7 +72,6 @@ bool threadpool_run(void *(*startRoutine)(void *), void *arg)
 			free( entry );
 			return false;
 		}
-		printf( "[DEBUG] Thread created!\n" );
 	}
 	entry->next = NULL;
 	entry->startRoutine = startRoutine;
@@ -125,7 +124,6 @@ static void *threadpool_worker(void *entryPtr)
 	}
 	signal_close( entry->signalFd );
 	free( entry );
-	printf(" [DEBUG] Thread killed!\n" );
 	return NULL;
 }
 
diff --git a/src/server/uplink.c b/src/server/uplink.c
index cc82fe0..fdc4b27 100644
--- a/src/server/uplink.c
+++ b/src/server/uplink.c
@@ -43,7 +43,8 @@ bool uplink_init(dnbd3_image_t *image, int sock, dnbd3_host_t *host)
 	assert( image != NULL );
 	spin_lock( &image->lock );
 	if ( image->uplink != NULL ) {
-		goto failure;
+		spin_unlock( &image->lock );
+		return true; // There's already an uplink, so should we consider this success or failure?
 	}
 	if ( image->cache_map == NULL ) {
 		memlogf( "[WARNING] Uplink was requested for image %s, but it is already complete", image->lower_name );
@@ -66,47 +67,49 @@ bool uplink_init(dnbd3_image_t *image, int sock, dnbd3_host_t *host)
 	link->recvBufferLen = 0;
 	link->shutdown = false;
 	spin_init( &link->queueLock, PTHREAD_PROCESS_PRIVATE );
-	if ( 0 != thread_create( &(link->thread), NULL, &uplink_mainloop, (void *)(uintptr_t)link ) ) {
-		memlogf( "[ERROR] Could not start thread for new client." );
+	if ( 0 != thread_create( &(link->thread), NULL, &uplink_mainloop, (void *)link ) ) {
+		memlogf( "[ERROR] Could not start thread for new uplink." );
 		goto failure;
 	}
 	spin_unlock( &image->lock );
 	return true;
 failure: ;
-	if ( link != NULL ) free( link );
-	link = image->uplink = NULL;
+	if ( link != NULL ) {
+		free( link );
+		link = image->uplink = NULL;
+	}
 	spin_unlock( &image->lock );
 	return false;
 }
 
 /**
  * Locks on image.lock, uplink.lock
- * Sets image->uplink to NULL while locked, so
- * calling it multiple times, even concurrently, will
+ * Calling it multiple times, even concurrently, will
  * not break anything.
  */
 void uplink_shutdown(dnbd3_image_t *image)
 {
+	bool join = false;
+	pthread_t thread;
 	assert( image != NULL );
 	spin_lock( &image->lock );
-	if ( image->uplink == NULL || image->uplink->shutdown ) {
+	if ( image->uplink == NULL ) {
 		spin_unlock( &image->lock );
 		return;
 	}
 	dnbd3_connection_t * const uplink = image->uplink;
 	spin_lock( &uplink->queueLock );
-	if ( uplink->shutdown ) {
-		spin_unlock( &uplink->queueLock );
-		spin_unlock( &image->lock );
-		return;
+	if ( !uplink->shutdown ) {
+		uplink->shutdown = true;
+		signal_call( uplink->signal );
+		thread = uplink->thread;
+		join = true;
 	}
-	image->uplink = NULL;
-	uplink->shutdown = true;
-	signal_call( uplink->signal );
-	pthread_t thread = uplink->thread;
 	spin_unlock( &uplink->queueLock );
 	spin_unlock( &image->lock );
-	thread_join( thread, NULL );
+	if ( join ) thread_join( thread, NULL );
+	while ( image->uplink != NULL )
+		usleep( 10000 );
 }
 
 /**
@@ -140,6 +143,11 @@ bool uplink_request(dnbd3_client_t *client, uint64_t handle, uint64_t start, uin
 		return false;
 	}
 	dnbd3_connection_t * const uplink = client->image->uplink;
+	if ( uplink->shutdown ) {
+		spin_unlock( &client->image->lock );
+		printf( "[DEBUG] Uplink request for image with uplink shutting down (%s)\n", client->image->lower_name );
+		return false;
+	}
 	// Check if the client is the same host as the uplink. If so assume this is a circular proxy chain
 	if ( isSameAddress( &uplink->currentServer, &client->host ) ) {
 		spin_unlock( &client->image->lock );
@@ -253,7 +261,7 @@ static void* uplink_mainloop(void *data)
 			link->image->working = true;
 			buffer[0] = '@';
 			if ( host_to_string( &link->currentServer, buffer + 1, sizeof(buffer) - 1 ) ) {
-				printf( "[DEBUG] Now connected to %s\n", buffer + 1 );
+				printf( "[DEBUG] (Uplink %s) Now connected to %s\n", link->image->lower_name, buffer + 1 );
 				setThreadName( buffer );
 			}
 			// Re-send all pending requests