Browse Source

H2O: Fix a couple of issues (#2974)

* H2O: Fix a file descriptor leak found by Coverity Scan

* H2O: Improve the shutdown process

This commit makes the process of updating the application easier.
It also adds some explanatory comments.
Anton Kirilov 8 years ago
parent
commit
1cd9ef4dbf

+ 5 - 6
frameworks/C/h2o/src/database.c

@@ -407,7 +407,7 @@ static void start_database_connect(thread_context_t *ctx, db_conn_t *db_conn)
 
 
 	if (flags < 0 || fcntl(sd, F_SETFD, flags | FD_CLOEXEC)) {
 	if (flags < 0 || fcntl(sd, F_SETFD, flags | FD_CLOEXEC)) {
 		STANDARD_ERROR("fcntl");
 		STANDARD_ERROR("fcntl");
-		goto error_dup;
+		goto error_fcntl;
 	}
 	}
 
 
 	db_conn->sock = h2o_evloop_socket_create(ctx->event_loop.h2o_ctx.loop,
 	db_conn->sock = h2o_evloop_socket_create(ctx->event_loop.h2o_ctx.loop,
@@ -425,12 +425,11 @@ static void start_database_connect(thread_context_t *ctx, db_conn_t *db_conn)
 		h2o_socket_notify_write(db_conn->sock, on_database_write_ready);
 		h2o_socket_notify_write(db_conn->sock, on_database_write_ready);
 		return;
 		return;
 	}
 	}
-	else {
-		errno = ENOMEM;
-		STANDARD_ERROR("h2o_evloop_socket_create");
-		close(sd);
-	}
 
 
+	errno = ENOMEM;
+	STANDARD_ERROR("h2o_evloop_socket_create");
+error_fcntl:
+	close(sd);
 error_dup:
 error_dup:
 	PQfinish(db_conn->conn);
 	PQfinish(db_conn->conn);
 error_connect:
 error_connect:

+ 28 - 5
frameworks/C/h2o/src/event_loop.c

@@ -175,10 +175,20 @@ static void process_messages(h2o_multithread_receiver_t *receiver, h2o_linklist_
 	                                                                         h2o_receiver,
 	                                                                         h2o_receiver,
 	                                                                         receiver);
 	                                                                         receiver);
 
 
-	if (global_thread_data->ctx->event_loop.h2o_https_socket)
+	// Close the listening sockets immediately, so that if another instance of
+	// the application is started before the current one exits (e.g. when doing
+	// an update), it will accept all incoming connections.
+	if (global_thread_data->ctx->event_loop.h2o_https_socket) {
 		h2o_socket_read_stop(global_thread_data->ctx->event_loop.h2o_https_socket);
 		h2o_socket_read_stop(global_thread_data->ctx->event_loop.h2o_https_socket);
+		h2o_socket_close(global_thread_data->ctx->event_loop.h2o_https_socket);
+		global_thread_data->ctx->event_loop.h2o_https_socket = NULL;
+	}
 
 
-	h2o_socket_read_stop(global_thread_data->ctx->event_loop.h2o_socket);
+	if (global_thread_data->ctx->event_loop.h2o_socket) {
+		h2o_socket_read_stop(global_thread_data->ctx->event_loop.h2o_socket);
+		h2o_socket_close(global_thread_data->ctx->event_loop.h2o_socket);
+		global_thread_data->ctx->event_loop.h2o_socket = NULL;
+	}
 }
 }
 
 
 static void shutdown_server(h2o_socket_t *listener, const char *err)
 static void shutdown_server(h2o_socket_t *listener, const char *err)
@@ -191,10 +201,21 @@ static void shutdown_server(h2o_socket_t *listener, const char *err)
 		                                                      listener->data);
 		                                                      listener->data);
 
 
 		ctx->global_data->shutdown = true;
 		ctx->global_data->shutdown = true;
-		h2o_socket_read_stop(ctx->event_loop.h2o_socket);
 
 
-		if (ctx->event_loop.h2o_https_socket)
+		// Close the listening sockets immediately, so that if another instance
+		// of the application is started before the current one exits (e.g. when
+		// doing an update), it will accept all incoming connections.
+		if (ctx->event_loop.h2o_https_socket) {
 			h2o_socket_read_stop(ctx->event_loop.h2o_https_socket);
 			h2o_socket_read_stop(ctx->event_loop.h2o_https_socket);
+			h2o_socket_close(ctx->event_loop.h2o_https_socket);
+			ctx->event_loop.h2o_https_socket = NULL;
+		}
+
+		if (ctx->event_loop.h2o_socket) {
+			h2o_socket_read_stop(ctx->event_loop.h2o_socket);
+			h2o_socket_close(ctx->event_loop.h2o_socket);
+			ctx->event_loop.h2o_socket = NULL;
+		}
 
 
 		for (size_t i = ctx->config->thread_num - 1; i > 0; i--)
 		for (size_t i = ctx->config->thread_num - 1; i > 0; i--)
 			h2o_multithread_send_message(&ctx->global_thread_data[i].h2o_receiver, NULL);
 			h2o_multithread_send_message(&ctx->global_thread_data[i].h2o_receiver, NULL);
@@ -235,8 +256,10 @@ void free_event_loop(event_loop_t *event_loop, h2o_multithread_receiver_t *h2o_r
 	if (event_loop->h2o_https_socket)
 	if (event_loop->h2o_https_socket)
 		h2o_socket_close(event_loop->h2o_https_socket);
 		h2o_socket_close(event_loop->h2o_https_socket);
 
 
+	if (event_loop->h2o_socket)
+		h2o_socket_close(event_loop->h2o_socket);
+
 	h2o_multithread_unregister_receiver(event_loop->h2o_ctx.queue, h2o_receiver);
 	h2o_multithread_unregister_receiver(event_loop->h2o_ctx.queue, h2o_receiver);
-	h2o_socket_close(event_loop->h2o_socket);
 
 
 	h2o_loop_t * const loop = event_loop->h2o_ctx.loop;
 	h2o_loop_t * const loop = event_loop->h2o_ctx.loop;
 
 

+ 2 - 0
frameworks/C/h2o/src/main.c

@@ -333,6 +333,8 @@ int main(int argc, char *argv[])
 			setup_process();
 			setup_process();
 			start_threads(global_data.global_thread_data);
 			start_threads(global_data.global_thread_data);
 			initialize_thread_context(global_data.global_thread_data, true, &ctx);
 			initialize_thread_context(global_data.global_thread_data, true, &ctx);
+			// This is just an optimization, so that the application does not try to
+			// establish database connections in the middle of servicing requests.
 			connect_to_database(&ctx);
 			connect_to_database(&ctx);
 			event_loop(&ctx);
 			event_loop(&ctx);
 			// Even though this is global data, we need to close
 			// Even though this is global data, we need to close

+ 2 - 0
frameworks/C/h2o/src/thread.c

@@ -41,6 +41,8 @@ static void *run_thread(void *arg)
 	thread_context_t ctx;
 	thread_context_t ctx;
 
 
 	initialize_thread_context(arg, false, &ctx);
 	initialize_thread_context(arg, false, &ctx);
+	// This is just an optimization, so that the application does not try to
+	// establish database connections in the middle of servicing requests.
 	connect_to_database(&ctx);
 	connect_to_database(&ctx);
 	event_loop(&ctx);
 	event_loop(&ctx);
 	free_thread_context(&ctx);
 	free_thread_context(&ctx);