From e5b04c8de83aa17e324bae95f18a822931fb8078 Mon Sep 17 00:00:00 2001 From: Patrik Flykt Date: Wed, 9 Apr 2014 13:12:07 +0300 Subject: [PATCH] sd-dhcp-client: Add reference counting for DHCP The DHCP library user can decide to free the DHCP client any time the callback is called. After the callback has been called, other computations may still be needed - the best example being a full restart of the DHCP procedure in case of lease expiry. Fix this by introducing proper reference counting. Properly handle a returned NULL from the notify and stop functions if the DHCP client was freed. --- src/libsystemd-network/sd-dhcp-client.c | 93 ++++++++++++++++------- src/libsystemd-network/test-dhcp-client.c | 4 +- src/network/networkd-link.c | 2 +- src/systemd/sd-dhcp-client.h | 5 +- 4 files changed, 72 insertions(+), 32 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index 392e294ae43..512bc9368df 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -28,6 +28,7 @@ #include "util.h" #include "list.h" +#include "refcnt.h" #include "dhcp-protocol.h" #include "dhcp-internal.h" @@ -35,6 +36,8 @@ #include "sd-dhcp-client.h" struct sd_dhcp_client { + RefCount n_ref; + DHCPState state; sd_event *event; int event_priority; @@ -77,6 +80,7 @@ static int client_receive_message_raw(sd_event_source *s, int fd, uint32_t revents, void *userdata); static int client_receive_message_udp(sd_event_source *s, int fd, uint32_t revents, void *userdata); +static sd_dhcp_client *client_stop(sd_dhcp_client *client, int error); int sd_dhcp_client_set_callback(sd_dhcp_client *client, sd_dhcp_client_cb_t cb, void *userdata) { @@ -155,10 +159,13 @@ int sd_dhcp_client_set_mac(sd_dhcp_client *client, if (client->state != DHCP_STATE_INIT) { log_dhcp_client(client, "Changing MAC address on running DHCP " "client, restarting"); - sd_dhcp_client_stop(client); need_restart = true; + client = client_stop(client, DHCP_EVENT_STOP); } + if (!client) + return 0; + memcpy(&client->client_id.mac_addr, addr, ETH_ALEN); client->client_id.type = 0x01; @@ -182,11 +189,14 @@ int sd_dhcp_client_get_lease(sd_dhcp_client *client, sd_dhcp_lease **ret) { return 0; } -static int client_notify(sd_dhcp_client *client, int event) { - if (client->cb) +static sd_dhcp_client *client_notify(sd_dhcp_client *client, int event) { + if (client->cb) { + client = sd_dhcp_client_ref(client); client->cb(client, event, client->userdata); + client = sd_dhcp_client_unref(client); + } - return 0; + return client; } static int client_initialize(sd_dhcp_client *client) { @@ -214,16 +224,17 @@ static int client_initialize(sd_dhcp_client *client) { return 0; } -static int client_stop(sd_dhcp_client *client, int error) { - assert_return(client, -EINVAL); +static sd_dhcp_client *client_stop(sd_dhcp_client *client, int error) { + assert_return(client, NULL); - client_notify(client, error); + log_dhcp_client(client, "STOPPED %d", error); - client_initialize(client); + client = client_notify(client, error); - log_dhcp_client(client, "STOPPED"); + if (client) + client_initialize(client); - return 0; + return client; } static int client_message_init(sd_dhcp_client *client, DHCPMessage *message, @@ -617,11 +628,13 @@ static int client_timeout_expire(sd_event_source *s, uint64_t usec, log_dhcp_client(client, "EXPIRED"); - client_notify(client, DHCP_EVENT_EXPIRED); + client = client_notify(client, DHCP_EVENT_EXPIRED); - /* start over as the lease was lost */ - client_initialize(client); - client_start(client); + /* lease was lost, start over if not freed */ + if (client) { + client_initialize(client); + client_start(client); + } return 0; } @@ -1031,8 +1044,11 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message, if (r < 0) goto error; - if (notify_event) - client_notify(client, notify_event); + if (notify_event) { + client = client_notify(client, notify_event); + if (!client) + return 0; + } client->receive_message = sd_event_source_unref(client->receive_message); @@ -1052,9 +1068,9 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message, error: if (r < 0 || r == DHCP_EVENT_NO_LEASE) - return client_stop(client, r); + client_stop(client, r); - return 0; + return r; } static int client_receive_message_udp(sd_event_source *s, int fd, @@ -1159,7 +1175,11 @@ int sd_dhcp_client_start(sd_dhcp_client *client) { } int sd_dhcp_client_stop(sd_dhcp_client *client) { - return client_stop(client, DHCP_EVENT_STOP); + assert_return(client, -EINVAL); + + client_stop(client, DHCP_EVENT_STOP); + + return 0; } int sd_dhcp_client_attach_event(sd_dhcp_client *client, sd_event *event, @@ -1197,19 +1217,35 @@ sd_event *sd_dhcp_client_get_event(sd_dhcp_client *client) { return client->event; } -void sd_dhcp_client_free(sd_dhcp_client *client) { - if (!client) - return; +sd_dhcp_client *sd_dhcp_client_ref(sd_dhcp_client *client) { + if (client) + assert_se(REFCNT_INC(client->n_ref) >= 2); + + return client; +} + +sd_dhcp_client *sd_dhcp_client_unref(sd_dhcp_client *client) { + if (client && REFCNT_DEC(client->n_ref) <= 0) { + log_dhcp_client(client, "UNREF"); - sd_dhcp_client_stop(client); - sd_dhcp_client_detach_event(client); + client_initialize(client); + + client->receive_message = + sd_event_source_unref(client->receive_message); + + sd_dhcp_client_detach_event(client); + + free(client->req_opts); + free(client); + + return NULL; + } - free(client->req_opts); - free(client); + return client; } -DEFINE_TRIVIAL_CLEANUP_FUNC(sd_dhcp_client*, sd_dhcp_client_free); -#define _cleanup_dhcp_client_free_ _cleanup_(sd_dhcp_client_freep) +DEFINE_TRIVIAL_CLEANUP_FUNC(sd_dhcp_client*, sd_dhcp_client_unref); +#define _cleanup_dhcp_client_free_ _cleanup_(sd_dhcp_client_unrefp) int sd_dhcp_client_new(sd_dhcp_client **ret) { _cleanup_dhcp_client_free_ sd_dhcp_client *client = NULL; @@ -1220,6 +1256,7 @@ int sd_dhcp_client_new(sd_dhcp_client **ret) { if (!client) return -ENOMEM; + client->n_ref = REFCNT_INIT; client->state = DHCP_STATE_INIT; client->index = -1; client->fd = -1; diff --git a/src/libsystemd-network/test-dhcp-client.c b/src/libsystemd-network/test-dhcp-client.c index 71b06b17a9b..9c316d75bcd 100644 --- a/src/libsystemd-network/test-dhcp-client.c +++ b/src/libsystemd-network/test-dhcp-client.c @@ -251,7 +251,7 @@ static void test_discover_message(sd_event *e) sd_event_run(e, (uint64_t) -1); sd_dhcp_client_stop(client); - sd_dhcp_client_free(client); + sd_dhcp_client_unref(client); test_fd[1] = safe_close(test_fd[1]); @@ -476,7 +476,7 @@ static void test_addr_acq(sd_event *e) { sd_dhcp_client_set_callback(client, NULL, NULL); sd_dhcp_client_stop(client); - sd_dhcp_client_free(client); + sd_dhcp_client_unref(client); test_fd[1] = safe_close(test_fd[1]); diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index f076525539b..2630b4625a7 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -80,7 +80,7 @@ void link_free(Link *link) { assert(link->manager); - sd_dhcp_client_free(link->dhcp_client); + sd_dhcp_client_unref(link->dhcp_client); sd_dhcp_lease_unref(link->dhcp_lease); sd_ipv4ll_free(link->ipv4ll); diff --git a/src/systemd/sd-dhcp-client.h b/src/systemd/sd-dhcp-client.h index a60bb5832f2..5818ec4d97b 100644 --- a/src/systemd/sd-dhcp-client.h +++ b/src/systemd/sd-dhcp-client.h @@ -54,7 +54,10 @@ int sd_dhcp_client_get_lease(sd_dhcp_client *client, sd_dhcp_lease **ret); int sd_dhcp_client_stop(sd_dhcp_client *client); int sd_dhcp_client_start(sd_dhcp_client *client); -void sd_dhcp_client_free(sd_dhcp_client *client); + +sd_dhcp_client *sd_dhcp_client_ref(sd_dhcp_client *client); +sd_dhcp_client *sd_dhcp_client_unref(sd_dhcp_client *client); + int sd_dhcp_client_new(sd_dhcp_client **ret); int sd_dhcp_client_attach_event(sd_dhcp_client *client, sd_event *event, int priority); -- 2.39.2