pxcloud/spice
5
0
Fork 0
mirror of https://gitlab.uni-freiburg.de/opensourcevdi/spice synced 2026-01-06 08:27:00 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Re-arrange channel client creation to avoid exposing client lock

Browse Source 
Instead of requiring the channel client to lock the client's lock,
re-arrange things so that all of the locking can be internal to
RedClient methods. So instead of having a pre-create validate function,
we simply move the check to red_client_add_channel() and return an error
if a channel already exists. This encapsulates the client implementation
better and also reduces code duplication in RedChannelClient and
DummyChannelClient.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This commit is contained in:
Jonathon Jongsma 2016-10-26 14:47:33 -05:00 committed by Frediano Ziglio
parent b3b5ab3663
commit 1f4705e130
4 changed files with 33 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
server/dummy-channel-client.c
View File

@ -35,46 +35,20 @@ struct DummyChannelClientPrivate
gboolean connected;
};
static int dummy_channel_client_pre_create_validate(RedChannel *channel, RedClient *client)
{
uint32_t type, id;
g_object_get(channel, "channel-type", &type, "id", &id, NULL);
if (red_client_get_channel(client, type, id)) {
spice_printerr("Error client %p: duplicate channel type %d id %d",
client, type, id);
return FALSE;
}
return TRUE;
}
static gboolean dummy_channel_client_initable_init(GInitable *initable,
GCancellable *cancellable,
GError **error)
{
GError *local_error = NULL;
DummyChannelClient *self = DUMMY_CHANNEL_CLIENT(initable);
RedChannelClient *rcc = RED_CHANNEL_CLIENT(self);
RedChannelClient *rcc = RED_CHANNEL_CLIENT(initable);
RedClient *client = red_channel_client_get_client(rcc);
RedChannel *channel = red_channel_client_get_channel(rcc);
uint32_t type, id;
g_object_get(channel, "channel-type", &type, "id", &id, NULL);
pthread_mutex_lock(&client->lock);
if (!dummy_channel_client_pre_create_validate(channel,
client)) {
g_set_error(&local_error,
SPICE_SERVER_ERROR,
SPICE_SERVER_ERROR_FAILED,
"Client %p: duplicate channel type %d id %d",
client, type, id);
goto cleanup;
}
red_channel_add_client(channel, rcc);
red_client_add_channel(client, rcc);
if (!red_client_add_channel(client, rcc, &local_error)) {
red_channel_remove_client(channel, rcc);
}
cleanup:
pthread_mutex_unlock(&client->lock);
if (local_error) {
g_warning("Failed to create channel client: %s", local_error->message);
g_propagate_error(error, local_error);

33
server/red-channel-client.c
View File

@ -862,18 +862,6 @@ static const SpiceDataHeaderOpaque mini_header_wrapper = {NULL, sizeof(SpiceMini
mini_header_get_msg_type,
mini_header_get_msg_size};
static int red_channel_client_pre_create_validate(RedChannel *channel, RedClient *client)
{
uint32_t type, id;
g_object_get(channel, "channel-type", &type, "id", &id, NULL);
if (red_client_get_channel(client, type, id)) {
spice_printerr("Error client %p: duplicate channel type %d id %d",
client, type, id);
return FALSE;
}
return TRUE;
}
static gboolean red_channel_client_initable_init(GInitable *initable,
GCancellable *cancellable,
GError **error)
@ -881,20 +869,6 @@ static gboolean red_channel_client_initable_init(GInitable *initable,
GError *local_error = NULL;
SpiceCoreInterfaceInternal *core;
RedChannelClient *self = RED_CHANNEL_CLIENT(initable);
pthread_mutex_lock(&self->priv->client->lock);
if (!red_channel_client_pre_create_validate(self->priv->channel, self->priv->client)) {
uint32_t id, type;
g_object_get(self->priv->channel,
"channel-type", &type,
"id", &id,
NULL);
g_set_error(&local_error,
SPICE_SERVER_ERROR,
SPICE_SERVER_ERROR_FAILED,
"Client %p: duplicate channel type %d id %d",
self->priv->client, type, id);
goto cleanup;
}
if (!red_channel_config_socket(self->priv->channel, self)) {
g_set_error_literal(&local_error,
@ -917,7 +891,7 @@ static gboolean red_channel_client_initable_init(GInitable *initable,
self->priv->latency_monitor.timer =
core->timer_add(core, red_channel_client_ping_timer, self);
if (!self->priv->client->during_target_migrate) {
if (!red_client_during_migrate_at_target(self->priv->client)) {
red_channel_client_start_ping_timer(self,
PING_TEST_IDLE_NET_TIMEOUT_MS);
}
@ -925,10 +899,11 @@ static gboolean red_channel_client_initable_init(GInitable *initable,
}
red_channel_add_client(self->priv->channel, self);
red_client_add_channel(self->priv->client, self);
if (!red_client_add_channel(self->priv->client, self, &local_error)) {
red_channel_remove_client(self->priv->channel, self);
}
cleanup:
pthread_mutex_unlock(&self->priv->client->lock);
if (local_error) {
g_warning("Failed to create channel client: %s", local_error->message);
g_propagate_error(error, local_error);

26
server/red-channel.c
View File

@ -797,15 +797,37 @@ RedChannelClient *red_client_get_channel(RedClient *client, int type, int id)
return ret;
}
/* client->lock should be locked */
void red_client_add_channel(RedClient *client, RedChannelClient *rcc)
gboolean red_client_add_channel(RedClient *client, RedChannelClient *rcc, GError **error)
{
uint32_t type, id;
RedChannel *channel;
gboolean result = TRUE;
spice_assert(rcc && client);
channel = red_channel_client_get_channel(rcc);
pthread_mutex_lock(&client->lock);
g_object_get(channel, "channel-type", &type, "id", &id, NULL);
if (red_client_get_channel(client, type, id)) {
g_set_error(error,
SPICE_SERVER_ERROR,
SPICE_SERVER_ERROR_FAILED,
"Client %p: duplicate channel type %d id %d",
client, type, id);
result = FALSE;
goto cleanup;
}
client->channels = g_list_prepend(client->channels, rcc);
if (client->during_target_migrate && client->seamless_migrate) {
if (red_channel_client_set_migration_seamless(rcc))
client->num_migrated_channels++;
}
cleanup:
pthread_mutex_unlock(&client->lock);
return result;
}
MainChannelClient *red_client_get_main(RedClient *client) {

2
server/red-channel.h
View File

@ -363,7 +363,7 @@ RedClient *red_client_ref(RedClient *client);
RedClient *red_client_unref(RedClient *client);
/* client->lock should be locked */
void red_client_add_channel(RedClient *client, RedChannelClient *rcc);
gboolean red_client_add_channel(RedClient *client, RedChannelClient *rcc, GError **error);
void red_client_remove_channel(RedChannelClient *rcc);
RedChannelClient *red_client_get_channel(RedClient *client, int type, int id);