[PATCH v2 wayland 04/11] connection: Make wl_closure_destroy() close fds of undispatched closures

Derek Foreman derekf at osg.samsung.com
Thu Apr 13 16:51:46 UTC 2017


When we have a closure that can't be dispatched for some reason, and it
contains file descriptors, we must close those descriptors to prevent
leaking them.

Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
---
 src/connection.c         | 35 ++++++++++++++++++++++++++++++++---
 src/wayland-client.c     | 12 ++++++------
 src/wayland-private.h    |  2 +-
 src/wayland-server.c     |  6 +++---
 tests/connection-test.c  | 12 ++++++------
 tests/os-wrappers-test.c |  4 ++--
 6 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index 934fddf..f2ebe06 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -548,6 +548,7 @@ wl_closure_marshal(struct wl_object *sender, uint32_t opcode,
 		errno = ENOMEM;
 		return NULL;
 	}
+	closure->message = NULL;
 
 	memcpy(closure->args, args, count * sizeof *args);
 
@@ -603,7 +604,7 @@ wl_closure_marshal(struct wl_object *sender, uint32_t opcode,
 err_null:
 	for (i = 0; i < fd_count; i++)
 		close(fds[i]);
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, false);
 	wl_log("error marshalling arguments for %s (signature %s): "
 	       "null value passed for arg %i\n", message->name,
 	       message->signature, i);
@@ -655,6 +656,7 @@ wl_connection_demarshal(struct wl_connection *connection,
 		wl_connection_consume(connection, size);
 		return NULL;
 	}
+	closure->message = NULL;
 
 	array_extra = closure->extra;
 	p = (uint32_t *)(closure->extra + num_arrays);
@@ -801,7 +803,7 @@ wl_connection_demarshal(struct wl_connection *connection,
  err:
 	for (i = 0; i < fd_count; i++)
 		close(fds[i]);
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, false);
 	wl_connection_consume(connection, size);
 
 	return NULL;
@@ -1244,8 +1246,35 @@ wl_closure_print(struct wl_closure *closure, struct wl_object *target, int send)
 	fprintf(stderr, ")\n");
 }
 
+static int
+wl_closure_close_fds(struct wl_closure *closure)
+{
+	const struct wl_message *message = closure->message;
+	uint32_t i, count;
+	struct argument_details arg;
+	const char *signature = message->signature;
+
+	count = arg_count_for_signature(signature);
+	for (i = 0; i < count; i++) {
+		signature = get_next_argument(signature, &arg);
+		if (arg.type == 'h')
+			close(closure->args[i].h);
+	}
+
+	return 0;
+}
+
 void
-wl_closure_destroy(struct wl_closure *closure)
+wl_closure_destroy(struct wl_closure *closure, bool dispatched)
 {
+	/* wl_closure_destroy has free() semantics */
+	if (!closure)
+		return;
+	/* If message is NULL then this closure failed during
+	 * creation, and any fd cleanup has been done by the
+	 * caller
+	 */
+	if (!dispatched && closure->message)
+		wl_closure_close_fds(closure);
 	free(closure);
 }
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 3d7361e..a77cf71 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -274,7 +274,7 @@ wl_event_queue_release(struct wl_event_queue *queue)
 		if (proxy_destroyed && !proxy->refcount)
 			free(proxy);
 
-		wl_closure_destroy(closure);
+		wl_closure_destroy(closure, false);
 	}
 }
 
@@ -658,7 +658,7 @@ wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *proxy,
 	if (wl_closure_send(closure, proxy->display->connection))
 		wl_abort("Error sending request: %s\n", strerror(errno));
 
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 
  err_unlock:
 	pthread_mutex_unlock(&proxy->display->mutex);
@@ -1243,12 +1243,12 @@ queue_event(struct wl_display *display, int len)
 		return -1;
 
 	if (create_proxies(proxy, closure) < 0) {
-		wl_closure_destroy(closure);
+		wl_closure_destroy(closure, false);
 		return -1;
 	}
 
 	if (wl_closure_lookup_objects(closure, &display->objects) != 0) {
-		wl_closure_destroy(closure);
+		wl_closure_destroy(closure, false);
 		return -1;
 	}
 
@@ -1291,7 +1291,7 @@ dispatch_event(struct wl_display *display, struct wl_event_queue *queue)
 		if (!proxy->refcount)
 			free(proxy);
 
-		wl_closure_destroy(closure);
+		wl_closure_destroy(closure, false);
 		return;
 	}
 
@@ -1311,7 +1311,7 @@ dispatch_event(struct wl_display *display, struct wl_event_queue *queue)
 				  &proxy->object, opcode, proxy->user_data);
 	}
 
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 
 	pthread_mutex_lock(&display->mutex);
 }
diff --git a/src/wayland-private.h b/src/wayland-private.h
index 434cb04..d906a6f 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -216,7 +216,7 @@ wl_closure_print(struct wl_closure *closure,
 		 struct wl_object *target, int send);
 
 void
-wl_closure_destroy(struct wl_closure *closure);
+wl_closure_destroy(struct wl_closure *closure, bool dispatched);
 
 extern wl_log_func_t wl_log_handler;
 
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 82a3b01..b24fb65 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -225,7 +225,7 @@ handle_array(struct wl_resource *resource, uint32_t opcode,
 
 	log_closure(resource, closure, true);
 
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 }
 
 WL_EXPORT void
@@ -395,7 +395,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 					       object->interface->name,
 					       object->id,
 					       message->name);
-			wl_closure_destroy(closure);
+			wl_closure_destroy(closure, false);
 			break;
 		}
 
@@ -410,7 +410,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 					    object, opcode);
 		}
 
-		wl_closure_destroy(closure);
+		wl_closure_destroy(closure, true);
 
 		if (client->error)
 			break;
diff --git a/tests/connection-test.c b/tests/connection-test.c
index 157e1bc..8e8fffe 100644
--- a/tests/connection-test.c
+++ b/tests/connection-test.c
@@ -211,7 +211,7 @@ marshal(struct marshal_data *data, const char *format, int size, ...)
 
 	assert(closure);
 	assert(wl_closure_send(closure, data->write_connection) == 0);
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 	assert(wl_connection_flush(data->write_connection) == size);
 	assert(read(data->s[0], data->buffer, sizeof data->buffer) == size);
 
@@ -298,7 +298,7 @@ expected_fail_marshal_send(struct marshal_data *data, int expected_error,
 	assert(wl_closure_send(closure, data->write_connection) < 0);
 	assert(errno == expected_error);
 
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, false);
 }
 
 TEST(connection_marshal_nullables)
@@ -405,7 +405,7 @@ demarshal(struct marshal_data *data, const char *format,
 					  size, &objects, &message);
 	assert(closure);
 	wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER, &object, 0, data);
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 }
 
 TEST(connection_demarshal)
@@ -475,7 +475,7 @@ marshal_demarshal(struct marshal_data *data,
 
 	assert(closure);
 	assert(wl_closure_send(closure, data->write_connection) == 0);
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 	assert(wl_connection_flush(data->write_connection) == size);
 
 	assert(wl_connection_read(data->read_connection) == size);
@@ -486,7 +486,7 @@ marshal_demarshal(struct marshal_data *data,
 					  size, &objects, &message);
 	assert(closure);
 	wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER, &object, 0, data);
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 }
 
 TEST(connection_marshal_demarshal)
@@ -592,7 +592,7 @@ marshal_helper(const char *format, void *handler, ...)
 	assert(closure);
 	done = 0;
 	wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER, &object, 0, &done);
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 	assert(done);
 }
 
diff --git a/tests/os-wrappers-test.c b/tests/os-wrappers-test.c
index 102622c..ddcd2f4 100644
--- a/tests/os-wrappers-test.c
+++ b/tests/os-wrappers-test.c
@@ -247,7 +247,7 @@ marshal_demarshal(struct marshal_data *data,
 
 	assert(closure);
 	assert(wl_closure_send(closure, data->write_connection) == 0);
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 	assert(wl_connection_flush(data->write_connection) == size);
 
 	assert(wl_connection_read(data->read_connection) == size);
@@ -258,7 +258,7 @@ marshal_demarshal(struct marshal_data *data,
 					  size, &objects, &message);
 	assert(closure);
 	wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER, &object, 0, data);
-	wl_closure_destroy(closure);
+	wl_closure_destroy(closure, true);
 }
 
 static void
-- 
2.11.0



More information about the wayland-devel mailing list