Replace some SecretSync with a sync implementation

The idea behind `SecretSync` is a nice thing: it allows use to re-use
our async implementations of methods for the synchronous versions. The
big problem with it, is that it makes debugging issues much harder
(especially with issues related to freezes) since things can seem to be
stuck in `poll()` somewhere.

Even though it adds quite a bit of code, I think it makes sense to
replace some instances with a proper synchronous implementation. Note
that we don't do this for all usages of `SecretSync` though, as some
things will need some kind of main loop interaction anyway (for example,
when waiting for a portal's Response signal)

Another nice advantage is that it's easier to follow the logic in the
sync code than the async version.
This commit is contained in:
Niels De Graef 2022-05-27 11:12:32 +02:00
parent a773060332
commit 3f4fdb0f69
6 changed files with 261 additions and 97 deletions

View File

@ -1705,26 +1705,19 @@ secret_collection_delete_sync (SecretCollection *self,
GCancellable *cancellable,
GError **error)
{
SecretSync *sync;
gboolean ret;
const char *object_path;
g_return_val_if_fail (SECRET_IS_COLLECTION (self), FALSE);
g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE);
g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
sync = _secret_sync_new ();
g_main_context_push_thread_default (sync->context);
secret_collection_delete (self, cancellable, _secret_sync_on_result, sync);
g_main_loop_run (sync->loop);
ret = secret_collection_delete_finish (self, sync->result, error);
g_main_context_pop_thread_default (sync->context);
_secret_sync_free (sync);
return ret;
object_path = g_dbus_proxy_get_object_path (G_DBUS_PROXY (self));
if (!_secret_service_delete_path_sync (self->pv->service, object_path, FALSE,
cancellable, error)) {
_secret_util_strip_remote_error (error);
return FALSE;
}
return TRUE;
}
/**

View File

@ -1411,6 +1411,42 @@ on_loads_secrets_session (GObject *source,
g_clear_object (&task);
}
static LoadsClosure *
load_secrets_prepare (GList *items)
{
LoadsClosure *ret;
GPtrArray *paths;
ret = g_new0 (LoadsClosure, 1);
ret->items = g_hash_table_new_full (g_str_hash, g_str_equal,
g_free, g_object_unref);
paths = g_ptr_array_new ();
for (GList *l = items; l != NULL; l = g_list_next (l)) {
const char *path;
if (secret_item_get_locked (l->data))
continue;
if (ret->service == NULL) {
ret->service = secret_item_get_service (l->data);
if (ret->service)
g_object_ref (ret->service);
}
path = g_dbus_proxy_get_object_path (l->data);
g_hash_table_insert (ret->items, g_strdup (path), g_object_ref (l->data));
g_ptr_array_add (paths, (void *) path);
}
ret->in = g_variant_new_objv ((const char * const *)paths->pdata, paths->len);
g_variant_ref_sink (ret->in);
g_ptr_array_free (paths, TRUE);
return ret;
}
/**
* secret_item_load_secrets:
* @items: (element-type Secret.Item): the items to retrieve secrets for
@ -1430,43 +1466,17 @@ secret_item_load_secrets (GList *items,
GAsyncReadyCallback callback,
gpointer user_data)
{
GTask *task;
LoadsClosure *loads;
GPtrArray *paths;
const gchar *path;
GList *l;
GTask *task;
for (GList *l = items; l != NULL; l = g_list_next (l))
g_return_if_fail (SECRET_IS_ITEM (l->data));
g_return_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable));
for (l = items; l != NULL; l = g_list_next (l))
g_return_if_fail (SECRET_IS_ITEM (l->data));
loads = load_secrets_prepare (items);
task = g_task_new (NULL, cancellable, callback, user_data);
g_task_set_source_tag (task, secret_item_load_secrets);
loads = g_new0 (LoadsClosure, 1);
loads->items = g_hash_table_new_full (g_str_hash, g_str_equal,
g_free, g_object_unref);
paths = g_ptr_array_new ();
for (l = items; l != NULL; l = g_list_next (l)) {
if (secret_item_get_locked (l->data))
continue;
if (loads->service == NULL) {
loads->service = secret_item_get_service (l->data);
if (loads->service)
g_object_ref (loads->service);
}
path = g_dbus_proxy_get_object_path (l->data);
g_hash_table_insert (loads->items, g_strdup (path), g_object_ref (l->data));
g_ptr_array_add (paths, (gpointer)path);
}
loads->in = g_variant_new_objv ((const gchar * const *)paths->pdata, paths->len);
g_variant_ref_sink (loads->in);
g_ptr_array_free (paths, TRUE);
g_task_set_task_data (task, loads, loads_closure_free);
if (loads->service) {
@ -1528,29 +1538,53 @@ secret_item_load_secrets_sync (GList *items,
GCancellable *cancellable,
GError **error)
{
SecretSync *sync;
gboolean ret;
GList *l;
for (l = items; l != NULL; l = g_list_next (l))
g_return_val_if_fail (SECRET_IS_ITEM (l->data), FALSE);
gboolean ret = TRUE;
LoadsClosure *loads;
const char *session_path;
GVariant *response;
g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE);
for (GList *l = items; l != NULL; l = g_list_next (l))
g_return_val_if_fail (SECRET_IS_ITEM (l->data), FALSE);
g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
sync = _secret_sync_new ();
g_main_context_push_thread_default (sync->context);
loads = load_secrets_prepare (items);
ret = secret_service_ensure_session_sync (loads->service, cancellable, error);
if (!ret) {
goto out;
}
secret_item_load_secrets (items, cancellable,
_secret_sync_on_result, sync);
session_path = secret_service_get_session_dbus_path (loads->service);
response = g_dbus_proxy_call_sync (G_DBUS_PROXY (loads->service), "GetSecrets",
g_variant_new ("(@aoo)", loads->in, session_path),
G_DBUS_CALL_FLAGS_NO_AUTO_START, -1,
cancellable, error);
g_main_loop_run (sync->loop);
if (response != NULL) {
GHashTable *with_paths;
GHashTableIter iter;
const char *path;
SecretValue *value;
SecretItem *item;
ret = secret_item_load_secrets_finish (sync->result, error);
with_paths = _secret_service_decode_get_secrets_all (loads->service, response);
g_return_val_if_fail (with_paths != NULL, FALSE);
g_main_context_pop_thread_default (sync->context);
_secret_sync_free (sync);
g_hash_table_iter_init (&iter, with_paths);
while (g_hash_table_iter_next (&iter, (void *) &path, (void *) &value)) {
item = g_hash_table_lookup (loads->items, path);
if (item != NULL)
_secret_item_set_cached_secret (item, value);
}
g_hash_table_unref (with_paths);
}
out:
if (response != NULL)
g_variant_unref (response);
_secret_util_strip_remote_error (error);
loads_closure_free (loads);
return ret;
}

View File

@ -432,28 +432,37 @@ secret_collection_search_for_dbus_paths_sync (SecretCollection *collection,
GCancellable *cancellable,
GError **error)
{
SecretSync *sync;
gchar **paths;
const char *schema_name = NULL;
GVariant *response = NULL;
char **paths = NULL;
g_return_val_if_fail (SECRET_IS_COLLECTION (collection), NULL);
g_return_val_if_fail (attributes != NULL, NULL);
g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), NULL);
g_return_val_if_fail (error == NULL || *error == NULL, NULL);
sync = _secret_sync_new ();
g_main_context_push_thread_default (sync->context);
/* Warnings raised already */
if (schema != NULL && !_secret_attributes_validate (schema, attributes, G_STRFUNC, TRUE))
return NULL;
secret_collection_search_for_dbus_paths (collection, schema, attributes, cancellable,
_secret_sync_on_result, sync);
if (schema != NULL && !(schema->flags & SECRET_SCHEMA_DONT_MATCH_NAME))
schema_name = schema->name;
g_main_loop_run (sync->loop);
paths = secret_collection_search_for_dbus_paths_finish (collection, sync->result, error);
response =
g_dbus_proxy_call_sync (G_DBUS_PROXY (collection), "SearchItems",
g_variant_new ("(@a{ss})",
_secret_attributes_to_variant (attributes, schema_name)),
G_DBUS_CALL_FLAGS_NONE, -1, cancellable, error);
if (response == NULL) {
_secret_util_strip_remote_error (error);
return FALSE;
}
g_main_context_pop_thread_default (sync->context);
_secret_sync_free (sync);
g_variant_get (response, "(^ao)", &paths);
g_variant_unref (response);
return paths;
return g_steal_pointer (&paths);
}
/**
@ -831,28 +840,37 @@ secret_service_get_secret_for_dbus_path_sync (SecretService *self,
GCancellable *cancellable,
GError **error)
{
SecretSync *sync;
SecretValue *value;
const char *session = NULL;
GVariant *response = NULL;
SecretValue *ret = NULL;
g_return_val_if_fail (SECRET_IS_SERVICE (self), NULL);
g_return_val_if_fail (item_path != NULL, NULL);
g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), NULL);
g_return_val_if_fail (error == NULL || *error == NULL, NULL);
sync = _secret_sync_new ();
g_main_context_push_thread_default (sync->context);
if (!secret_service_ensure_session_sync (self, cancellable, error)) {
_secret_util_strip_remote_error (error);
return NULL;
}
secret_service_get_secret_for_dbus_path (self, item_path, cancellable,
_secret_sync_on_result, sync);
session = secret_service_get_session_dbus_path (self);
response =
g_dbus_proxy_call_sync (G_DBUS_PROXY (self), "GetSecrets",
g_variant_new ("(@aoo)",
g_variant_new_objv (&item_path, 1),
session),
G_DBUS_CALL_FLAGS_NO_AUTO_START, -1,
cancellable, error);
if (response) {
ret = _secret_service_decode_get_secrets_first (self, response);
} else {
_secret_util_strip_remote_error (error);
}
g_main_loop_run (sync->loop);
g_clear_pointer (&response, g_variant_unref);
value = secret_service_get_secret_for_dbus_path_finish (self, sync->result, error);
g_main_context_pop_thread_default (sync->context);
_secret_sync_free (sync);
return value;
return ret;
}
/**
@ -1527,6 +1545,63 @@ _secret_service_delete_path_finish (SecretService *self,
return TRUE;
}
gboolean
_secret_service_delete_path_sync (SecretService *self,
const char *object_path,
gboolean is_an_item,
GCancellable *cancellable,
GError **error)
{
GError *err = NULL;
GVariant *prompt_variant = NULL;
gboolean ret = TRUE;
g_return_val_if_fail (SECRET_IS_SERVICE (self), FALSE);
g_return_val_if_fail (object_path != NULL, FALSE);
g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE);
prompt_variant =
g_dbus_connection_call_sync (g_dbus_proxy_get_connection (G_DBUS_PROXY (self)),
g_dbus_proxy_get_name (G_DBUS_PROXY (self)),
object_path,
is_an_item ? SECRET_ITEM_INTERFACE : SECRET_COLLECTION_INTERFACE,
"Delete", g_variant_new ("()"), G_VARIANT_TYPE ("(o)"),
G_DBUS_CALL_FLAGS_NO_AUTO_START, -1,
cancellable,
&err);
if (err == NULL) {
const char *prompt_path;
g_variant_get (prompt_variant, "(&o)", &prompt_path);
/* show a prompt if needed */
if (!_secret_util_empty_path (prompt_path)) {
SecretPrompt *prompt = NULL;
GVariant *prompt_result = NULL;
prompt = _secret_prompt_instance (self, prompt_path);
prompt_result = secret_service_prompt_sync (self,
prompt,
cancellable,
NULL,
&err);
ret = (err != NULL);
g_clear_pointer (&prompt_result, g_variant_unref);
g_clear_object (&prompt);
}
} else {
ret = FALSE;
}
g_clear_pointer (&prompt_variant, g_variant_unref);
if (err) {
_secret_util_strip_remote_error (&err);
g_propagate_error (error, err);
}
return ret;
}
/**
* secret_service_delete_item_dbus_path: (skip)
* @self: the secret service

View File

@ -140,6 +140,12 @@ gboolean _secret_service_delete_path_finish (SecretService *se
GAsyncResult *result,
GError **error);
gboolean _secret_service_delete_path_sync (SecretService *self,
const char *object_path,
gboolean is_an_item,
GCancellable *cancellable,
GError **error);
void _secret_service_search_for_paths_variant (SecretService *self,
GVariant *attributes,
GCancellable *cancellable,
@ -198,6 +204,10 @@ void _secret_session_open (SecretService *se
gboolean _secret_session_open_finish (GAsyncResult *result,
GError **error);
gboolean _secret_session_open_sync (SecretService *service,
GCancellable *cancellable,
GError **error);
GVariant * _secret_session_encode_secret (SecretSession *session,
SecretValue *value);

View File

@ -1544,27 +1544,20 @@ secret_service_ensure_session_sync (SecretService *self,
GCancellable *cancellable,
GError **error)
{
SecretSync *sync;
gboolean ret;
SecretSession *session;
g_return_val_if_fail (SECRET_IS_SERVICE (self), FALSE);
g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE);
g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
sync = _secret_sync_new ();
g_main_context_push_thread_default (sync->context);
g_mutex_lock (&self->pv->mutex);
session = self->pv->session;
g_mutex_unlock (&self->pv->mutex);
secret_service_ensure_session (self, cancellable,
_secret_sync_on_result, sync);
if (session != NULL)
return TRUE;
g_main_loop_run (sync->loop);
ret = secret_service_ensure_session_finish (self, sync->result, error);
g_main_context_pop_thread_default (sync->context);
_secret_sync_free (sync);
return ret;
return _secret_session_open_sync (self, cancellable, error);
}
static SecretCollection *

View File

@ -363,6 +363,65 @@ _secret_session_open_finish (GAsyncResult *result,
return TRUE;
}
gboolean
_secret_session_open_sync (SecretService *service,
GCancellable *cancellable,
GError **error)
{
SecretSession *session = NULL;
GVariant *params;
GVariant *result = NULL;
session = g_new0 (SecretSession, 1);
#ifdef WITH_CRYPTO
params = request_open_session_aes (session);
result = g_dbus_proxy_call_sync (G_DBUS_PROXY (service), "OpenSession",
params,
G_DBUS_CALL_FLAGS_NONE, -1,
cancellable, error);
if (result != NULL) {
if (response_open_session_aes (session, result)) {
_secret_service_take_session (service, g_steal_pointer (&session));
goto out;
} else {
g_set_error (error, SECRET_ERROR, SECRET_ERROR_PROTOCOL,
_("Couldnt communicate with the secret storage"));
goto out;
}
} else {
/* AES session not supported, request a plain session */
if (g_error_matches (*error, G_DBUS_ERROR, G_DBUS_ERROR_NOT_SUPPORTED)) {
g_clear_error (error);
/* Fall through */
} else {
goto out;
}
}
#endif
params = request_open_session_plain (session);
result = g_dbus_proxy_call_sync (G_DBUS_PROXY (service), "OpenSession",
params,
G_DBUS_CALL_FLAGS_NONE, -1,
cancellable, error);
if (result != NULL) {
if (response_open_session_plain (session, result)) {
_secret_service_take_session (service, g_steal_pointer (&session));
goto out;
} else {
g_set_error (error, SECRET_ERROR, SECRET_ERROR_PROTOCOL,
_("Couldnt communicate with the secret storage"));
goto out;
}
}
out:
_secret_util_strip_remote_error (error);
g_clear_object (&session);
return result != NULL;
}
#ifdef WITH_CRYPTO
static gboolean