gsocketclient: emit RESOLVING/RESOLVED events only once

GSocketAddressEnumerator encapsulates the details of how DNS happens, so
we don't have to think about it. But we may have taken encapsulation a
bit too far, here. Usually, we resolve a domain name to a list of IPv4
and IPv6 addresses. Then we go through each address in the list and try
to connect to it. Name resolution happens exactly once, at the start.
It doesn't happen each time we enumerate the enumerator. In theory, it
*could*, because we've designed these APIs to be agnostic of underlying
implementation details like DNS and network protocols. But in practice,
we know that's not really what's happening. It's weird to say that we
are RESOLVING what we know to be the same name multiple times. Behind
the scenes, we're not doing that.

This also fixes #1994, where enumeration can end with a RESOLVING event,
even though this is supposed to be the first event rather than the last.
I thought this would be hard to fix, even requiring new public API in
GSocketAddressEnumerator to peek ahead to see if the next enumeration is
going to return NULL. Then I decided we should just fake it: always emit
both RESOLVING and RESOLVED at the same time right after each
enumeration. Finally, I realized we can emit them at the correct time if
we simply assume resolving only happens the first time. This seems like
the most elegant of the possible solutions.

Now, this is a behavior change, and arguably an API break, but it should
align better with reasonable expectations of how GSocketClientEvent
ought to work. I don't expect it to break anything besides tests that
check which order GSocketClientEvent events are emitted in. (Currently,
libsoup has such tests, which will need to be updated.) Ideally we would
have GLib-level tests as well, but in a concession to pragmatism, it's a
lot easier to keep network tests in libsoup.
This commit is contained in:
Michael Catanzaro 2020-10-06 15:39:45 -05:00
parent 290d5722be
commit f0a7b14780

View File

@ -991,6 +991,7 @@ g_socket_client_connect (GSocketClient *client,
{
GIOStream *connection = NULL;
GSocketAddressEnumerator *enumerator = NULL;
gboolean ever_resolved = FALSE;
GError *last_error, *tmp_error;
last_error = NULL;
@ -1025,10 +1026,20 @@ g_socket_client_connect (GSocketClient *client,
}
tmp_error = NULL;
g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVING,
connectable, NULL);
if (!ever_resolved)
{
g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVING,
connectable, NULL);
}
address = g_socket_address_enumerator_next (enumerator, cancellable,
&tmp_error);
if (!ever_resolved)
{
g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVED,
connectable, NULL);
ever_resolved = TRUE;
}
if (address == NULL)
{
@ -1046,8 +1057,6 @@ g_socket_client_connect (GSocketClient *client,
_("Unknown error on connect"));
break;
}
g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVED,
connectable, NULL);
using_proxy = (G_IS_PROXY_ADDRESS (address) &&
client->priv->enable_proxy);
@ -1509,7 +1518,8 @@ enumerator_next_async (GSocketClientAsyncConnectData *data,
if (add_task_ref)
g_object_ref (data->task);
g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVING, data->connectable, NULL);
if (!data->enumerated_at_least_once)
g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVING, data->connectable, NULL);
g_debug ("GSocketClient: Starting new address enumeration");
g_socket_address_enumerator_next_async (data->enumerator,
data->enumeration_cancellable,
@ -1883,10 +1893,13 @@ g_socket_client_enumerator_callback (GObject *object,
return;
}
data->enumerated_at_least_once = TRUE;
g_debug ("GSocketClient: Address enumeration succeeded");
g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVED,
data->connectable, NULL);
if (!data->enumerated_at_least_once)
{
g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVED,
data->connectable, NULL);
data->enumerated_at_least_once = TRUE;
}
g_clear_error (&data->last_error);