From 34e4f1a9b5cce3b23a46093d8dbb4db550f822f7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 18 Sep 2024 13:41:59 -0400 Subject: [PATCH] curl: Only free multi handle as last action This is my best current guess as to the cause of https://github.com/ostreedev/ostree/issues/3299 If the fetcher gets torn down while it has outstanding requests in flight, I think we'll end up in some code paths calling into libcurl to cancel things. It might be that prior to the curl change, while the handle was invalid it was "valid enough" for that to work. Clearly moving the multi cleanup to be the last step is a safe thing, and it might fix the bug, so let's do that. Signed-off-by: Colin Walters --- src/libostree/ostree-fetcher-curl.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index d6902893ff..8209985249 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -180,7 +180,6 @@ _ostree_fetcher_finalize (GObject *object) { OstreeFetcher *self = OSTREE_FETCHER (object); - curl_multi_cleanup (self->multi); g_free (self->remote_name); g_free (self->tls_ca_db_path); g_free (self->tls_client_cert_path); @@ -195,6 +194,11 @@ _ostree_fetcher_finalize (GObject *object) if (self->mainctx) g_main_context_unref (self->mainctx); g_clear_pointer (&self->custom_user_agent, g_free); + // Do this last as it may be possible that some of the teardown functions + // invoked by callbacks above reference this. For example, + // self->outstanding_requests has a GTask whose cleanup invokes request_unref() + // which calls curl_easy_cleanup(). + curl_multi_cleanup (self->multi); G_OBJECT_CLASS (_ostree_fetcher_parent_class)->finalize (object); }