rpms/spice
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Update to spice 0.12.5

Browse Source
This commit is contained in:
Christophe Fergeau 2014-05-19 14:58:36 +02:00
parent bcb124ce4f
commit 8713af5567
12 changed files with 7 additions and 819 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored
View File

@ -17,3 +17,4 @@ spice-0.5.3.tar.bz2
/spice-0.12.2.tar.bz2
/spice-0.12.3.tar.bz2
/spice-0.12.4.tar.bz2
/spice-0.12.5.tar.bz2

96
0001-red_channel-prevent-adding-and-pushing-pipe-items-af.patch
View File

@ -1,96 +0,0 @@
From 47e722b85ccd0b6876ca189a3d6f6f05289fe3c3 Mon Sep 17 00:00:00 2001
From: Yonit Halperin <yhalperi@redhat.com>
Date: Wed, 24 Jul 2013 14:54:23 -0400
Subject: [PATCH 1/8] red_channel: prevent adding and pushing pipe items after
a channel_client has diconnected
Fixes: leaks of pipe items & "red_client_destroy: assertion `rcc->send_data.size == 0'"
red_channel_disconnect clears the pipe. It is called only once. After,
it was called, not items should be added to the pipe.
An example of when this assert can occur:
on_new_cursor_channel (red_worker.c), pushes 2 pipe items.
When it pushes the first pipe item, if the client has disconnected,
it can hit a socket error, and then, red_channel_client_disconnect is called.
The second call to adding a pipe item, will add the item to
the pipe. red_channel_client_pipe_add_type also calls
red_channel_client_push, which will update the send_data.size.
Then, the push will also hit a socket error, but red_channel_client_disconnect
won't clear the pending pipe item again, since it was already called.
When red_client_destory is called, we hit assertion `rcc->send_data.size
== 0'.
Note that if a pipe item is added to the pipe after
red_channel_client_disconnect was called, but without pushing it,
we should hit "spice_assert(rcc->pipe_size == 0)".
---
server/red_channel.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/server/red_channel.c b/server/red_channel.c
index 33af388..0d74413 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -1527,9 +1527,23 @@ void red_channel_pipe_item_init(RedChannel *channel, PipeItem *item, int type)
item->type = type;
}
-void red_channel_client_pipe_add(RedChannelClient *rcc, PipeItem *item)
+static inline int validate_pipe_add(RedChannelClient *rcc, PipeItem *item)
{
spice_assert(rcc && item);
+ if (SPICE_UNLIKELY(!red_channel_client_is_connected(rcc))) {
+ spice_debug("rcc is disconnected %p", rcc);
+ red_channel_client_release_item(rcc, item, FALSE);
+ return FALSE;
+ }
+ return TRUE;
+}
+
+void red_channel_client_pipe_add(RedChannelClient *rcc, PipeItem *item)
+{
+
+ if (!validate_pipe_add(rcc, item)) {
+ return;
+ }
rcc->pipe_size++;
ring_add(&rcc->pipe, &item->link);
}
@@ -1543,10 +1557,10 @@ void red_channel_client_pipe_add_push(RedChannelClient *rcc, PipeItem *item)
void red_channel_client_pipe_add_after(RedChannelClient *rcc,
PipeItem *item, PipeItem *pos)
{
- spice_assert(rcc);
spice_assert(pos);
- spice_assert(item);
-
+ if (!validate_pipe_add(rcc, item)) {
+ return;
+ }
rcc->pipe_size++;
ring_add_after(&item->link, &pos->link);
}
@@ -1560,14 +1574,18 @@ int red_channel_client_pipe_item_is_linked(RedChannelClient *rcc,
void red_channel_client_pipe_add_tail_no_push(RedChannelClient *rcc,
PipeItem *item)
{
- spice_assert(rcc);
+ if (!validate_pipe_add(rcc, item)) {
+ return;
+ }
rcc->pipe_size++;
ring_add_before(&item->link, &rcc->pipe);
}
void red_channel_client_pipe_add_tail(RedChannelClient *rcc, PipeItem *item)
{
- spice_assert(rcc);
+ if (!validate_pipe_add(rcc, item)) {
+ return;
+ }
rcc->pipe_size++;
ring_add_before(&item->link, &rcc->pipe);
red_channel_client_push(rcc);
--
1.8.3.1

96
0002-red_channel-add-ref-count-to-RedClient.patch
View File

@ -1,96 +0,0 @@
From aab45618cc12799d5f7351ef8832ae73b33057c7 Mon Sep 17 00:00:00 2001
From: Yonit Halperin <yhalperi@redhat.com>
Date: Fri, 26 Jul 2013 13:45:16 -0400
Subject: [PATCH 2/8] red_channel: add ref count to RedClient
---
server/red_channel.c | 23 ++++++++++++++++++++---
server/red_channel.h | 17 +++++++++++++++--
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/server/red_channel.c b/server/red_channel.c
index 0d74413..9168b8a 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -1932,10 +1932,29 @@ RedClient *red_client_new(int migrated)
pthread_mutex_init(&client->lock, NULL);
client->thread_id = pthread_self();
client->during_target_migrate = migrated;
+ client->refs = 1;
return client;
}
+RedClient *red_client_ref(RedClient *client)
+{
+ spice_assert(client);
+ client->refs++;
+ return client;
+}
+
+RedClient *red_client_unref(RedClient *client)
+{
+ if (!--client->refs) {
+ spice_debug("release client=%p", client);
+ pthread_mutex_destroy(&client->lock);
+ free(client);
+ return NULL;
+ }
+ return client;
+}
+
/* client mutex should be locked before this call */
static void red_channel_client_set_migration_seamless(RedChannelClient *rcc)
{
@@ -2012,9 +2031,7 @@ void red_client_destroy(RedClient *client)
spice_assert(rcc->send_data.size == 0);
red_channel_client_destroy(rcc);
}
-
- pthread_mutex_destroy(&client->lock);
- free(client);
+ red_client_unref(client);
}
/* client->lock should be locked */
diff --git a/server/red_channel.h b/server/red_channel.h
index ba299b6..0dd73ea 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -561,10 +561,25 @@ struct RedClient {
is called */
int seamless_migrate;
int num_migrated_channels; /* for seamless - number of channels that wait for migrate data*/
+ int refs;
};
RedClient *red_client_new(int migrated);
+/*
+ * disconnects all the client's channels (should be called from the client's thread)
+ */
+void red_client_destroy(RedClient *client);
+
+RedClient *red_client_ref(RedClient *client);
+
+/*
+ * releases the client resources when refs == 0.
+ * We assume the red_client_derstroy was called before
+ * we reached refs==0
+ */
+RedClient *red_client_unref(RedClient *client);
+
MainChannelClient *red_client_get_main(RedClient *client);
// main should be set once before all the other channels are created
void red_client_set_main(RedClient *client, MainChannelClient *mcc);
@@ -580,7 +595,5 @@ void red_client_semi_seamless_migrate_complete(RedClient *client); /* dst side *
int red_client_during_migrate_at_target(RedClient *client);
void red_client_migrate(RedClient *client);
-// disconnects all the client's channels (should be called from the client's thread)
-void red_client_destroy(RedClient *client);
#endif
--
1.8.3.1

51
0003-main_dispatcher-add-ref-count-protection-to-RedClien.patch
View File

@ -1,51 +0,0 @@
From 06ba03b7b32a2f0c7f78c82d8f399242526a0b45 Mon Sep 17 00:00:00 2001
From: Yonit Halperin <yhalperi@redhat.com>
Date: Fri, 26 Jul 2013 13:49:24 -0400
Subject: [PATCH 3/8] main_dispatcher: add ref count protection to RedClient
instances
---
server/main_dispatcher.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/server/main_dispatcher.c b/server/main_dispatcher.c
index e7a451a..bf160dd 100644
--- a/server/main_dispatcher.c
+++ b/server/main_dispatcher.c
@@ -97,6 +97,7 @@ static void main_dispatcher_handle_migrate_complete(void *opaque,
MainDispatcherMigrateSeamlessDstCompleteMessage *mig_complete = payload;
reds_on_client_seamless_migrate_complete(mig_complete->client);
+ red_client_unref(mig_complete->client);
}
static void main_dispatcher_handle_mm_time_latency(void *opaque,
@@ -104,6 +105,7 @@ static void main_dispatcher_handle_mm_time_latency(void *opaque,
{
MainDispatcherMmTimeLatencyMessage *msg = payload;
reds_set_client_mm_time_latency(msg->client, msg->latency);
+ red_client_unref(msg->client);
}
void main_dispatcher_seamless_migrate_dst_complete(RedClient *client)
@@ -115,7 +117,7 @@ void main_dispatcher_seamless_migrate_dst_complete(RedClient *client)
return;
}
- msg.client = client;
+ msg.client = red_client_ref(client);
dispatcher_send_message(&main_dispatcher.base, MAIN_DISPATCHER_MIGRATE_SEAMLESS_DST_COMPLETE,
&msg);
}
@@ -129,7 +131,7 @@ void main_dispatcher_set_mm_time_latency(RedClient *client, uint32_t latency)
return;
}
- msg.client = client;
+ msg.client = red_client_ref(client);
msg.latency = latency;
dispatcher_send_message(&main_dispatcher.base, MAIN_DISPATCHER_SET_MM_TIME_LATENCY,
&msg);
--
1.8.3.1

238
0004-decouple-disconnection-of-the-main-channel-from-clie.patch
View File

@ -1,238 +0,0 @@
From 8490f83e1f51ac8dbcac3c68c97827e0acb9ec4e Mon Sep 17 00:00:00 2001
From: Yonit Halperin <yhalperi@redhat.com>
Date: Thu, 25 Jul 2013 14:19:21 -0400
Subject: [PATCH 4/8] decouple disconnection of the main channel from client
destruction
Fixes rhbz#918169
Some channels make direct calls to reds/main_channel routines. If
these routines try to read/write to the socket, and they get socket
error, main_channel_client_on_disconnect is called, and triggers
red_client_destroy. In order to prevent accessing expired references
to RedClient, RedChannelClient, or other objects (inside the original call, after
red_client_destroy has been called) I made the call to
red_client_destroy asynchronous with respect to main_channel_client_on_disconnect.
I added MAIN_DISPATCHER_CLIENT_DISCONNECT to main_dispatcher.
main_channel_client_on_disconnect pushes this msg to the dispatcher,
instead of calling directly to reds_client_disconnect.
The patch uses RedClient ref-count in order to handle a case where
reds_client_disconnect is called directly (e.g., when a new client connects while
another one is connected), while there is already CLIENT_DISCONNECT msg
pending in the main_dispatcher.
Examples:
(1) snd_worker.c
snd_disconnect_channel()
channel->cleanup() //snd_playback_cleanup
reds_enable_mm_timer()
.
.
main_channel_push_multi_media_time()...socket_error
.
.
red_client_destory()
.
.
snd_disconnect_channel()
channel->cleanup()
celt051_encoder_destroy()
celt051_encoder_destory() // double release
Note that this bug could have been solved by changing the order of
calls: e.g., channel->stream = NULL before calling cleanup, and
some other changes + reference counting. However, I found other
places in the code with similar problems, and I looked for a general
solution, at least till we redesign red_channel to handle reference
counting more consistently.
(2) inputs_channel.c
inputs_connect()
main_channel_client_push_notify()...socket_error
.
.
red_client_destory()
.
.
red_channel_client_create() // refers to client which is already destroyed
(3) reds.c
reds_handle_main_link()
main_channel_push_init() ...socket error
.
.
red_client_destory()
.
.
main_channel_client_start_net_test(mcc) // refers to mcc which is already destroyed
This can explain the assert in rhbz#964136, comment #1 (but not the hang that occurred before).
---
server/main_channel.c | 9 +++++----
server/main_dispatcher.c | 37 +++++++++++++++++++++++++++++++++++++
server/main_dispatcher.h | 7 +++++++
server/reds.c | 1 +
server/reds.h | 2 ++
5 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/server/main_channel.c b/server/main_channel.c
index 233e633..fe032a6 100644
--- a/server/main_channel.c
+++ b/server/main_channel.c
@@ -46,6 +46,7 @@
#include "red_common.h"
#include "reds.h"
#include "migration_protocol.h"
+#include "main_dispatcher.h"
#define ZERO_BUF_SIZE 4096
@@ -175,13 +176,13 @@ int main_channel_is_connected(MainChannel *main_chan)
return red_channel_is_connected(&main_chan->base);
}
-// when disconnection occurs, let reds shutdown all channels. This will trigger the
-// real disconnection of main channel
+/*
+ * When the main channel is disconnected, disconnect the entire client.
+ */
static void main_channel_client_on_disconnect(RedChannelClient *rcc)
{
spice_printerr("rcc=%p", rcc);
- reds_client_disconnect(rcc->client);
-// red_channel_client_disconnect(rcc);
+ main_dispatcher_client_disconnect(rcc->client);
}
RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t connection_id)
diff --git a/server/main_dispatcher.c b/server/main_dispatcher.c
index bf160dd..dbe1037 100644
--- a/server/main_dispatcher.c
+++ b/server/main_dispatcher.c
@@ -41,6 +41,7 @@ enum {
MAIN_DISPATCHER_CHANNEL_EVENT = 0,
MAIN_DISPATCHER_MIGRATE_SEAMLESS_DST_COMPLETE,
MAIN_DISPATCHER_SET_MM_TIME_LATENCY,
+ MAIN_DISPATCHER_CLIENT_DISCONNECT,
MAIN_DISPATCHER_NUM_MESSAGES
};
@@ -59,6 +60,10 @@ typedef struct MainDispatcherMmTimeLatencyMessage {
uint32_t latency;
} MainDispatcherMmTimeLatencyMessage;
+typedef struct MainDispatcherClientDisconnectMessage {
+ RedClient *client;
+} MainDispatcherClientDisconnectMessage;
+
/* channel_event - calls core->channel_event, must be done in main thread */
static void main_dispatcher_self_handle_channel_event(
int event,
@@ -108,6 +113,16 @@ static void main_dispatcher_handle_mm_time_latency(void *opaque,
red_client_unref(msg->client);
}
+static void main_dispatcher_handle_client_disconnect(void *opaque,
+ void *payload)
+{
+ MainDispatcherClientDisconnectMessage *msg = payload;
+
+ spice_debug("client=%p", msg->client);
+ reds_client_disconnect(msg->client);
+ red_client_unref(msg->client);
+}
+
void main_dispatcher_seamless_migrate_dst_complete(RedClient *client)
{
MainDispatcherMigrateSeamlessDstCompleteMessage msg;
@@ -137,6 +152,20 @@ void main_dispatcher_set_mm_time_latency(RedClient *client, uint32_t latency)
&msg);
}
+void main_dispatcher_client_disconnect(RedClient *client)
+{
+ MainDispatcherClientDisconnectMessage msg;
+
+ if (!client->disconnecting) {
+ spice_debug("client %p", client);
+ msg.client = red_client_ref(client);
+ dispatcher_send_message(&main_dispatcher.base, MAIN_DISPATCHER_CLIENT_DISCONNECT,
+ &msg);
+ } else {
+ spice_debug("client %p already during disconnection", client);
+ }
+}
+
static void dispatcher_handle_read(int fd, int event, void *opaque)
{
Dispatcher *dispatcher = opaque;
@@ -144,6 +173,11 @@ static void dispatcher_handle_read(int fd, int event, void *opaque)
dispatcher_handle_recv_read(dispatcher);
}
+/*
+ * FIXME:
+ * Reds routines shouldn't be exposed. Instead reds.c should register the callbacks,
+ * and the corresponding operations should be made only via main_dispatcher.
+ */
void main_dispatcher_init(SpiceCoreInterface *core)
{
memset(&main_dispatcher, 0, sizeof(main_dispatcher));
@@ -160,4 +194,7 @@ void main_dispatcher_init(SpiceCoreInterface *core)
dispatcher_register_handler(&main_dispatcher.base, MAIN_DISPATCHER_SET_MM_TIME_LATENCY,
main_dispatcher_handle_mm_time_latency,
sizeof(MainDispatcherMmTimeLatencyMessage), 0 /* no ack */);
+ dispatcher_register_handler(&main_dispatcher.base, MAIN_DISPATCHER_CLIENT_DISCONNECT,
+ main_dispatcher_handle_client_disconnect,
+ sizeof(MainDispatcherClientDisconnectMessage), 0 /* no ack */);
}
diff --git a/server/main_dispatcher.h b/server/main_dispatcher.h
index 0c79ca8..522c7f9 100644
--- a/server/main_dispatcher.h
+++ b/server/main_dispatcher.h
@@ -7,6 +7,13 @@
void main_dispatcher_channel_event(int event, SpiceChannelEventInfo *info);
void main_dispatcher_seamless_migrate_dst_complete(RedClient *client);
void main_dispatcher_set_mm_time_latency(RedClient *client, uint32_t latency);
+/*
+ * Disconnecting the client is always executed asynchronously,
+ * in order to protect from expired references in the routines
+ * that triggered the client destruction.
+ */
+void main_dispatcher_client_disconnect(RedClient *client);
+
void main_dispatcher_init(SpiceCoreInterface *core);
#endif //MAIN_DISPATCHER_H
diff --git a/server/reds.c b/server/reds.c
index 30d0652..c66ddc4 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -537,6 +537,7 @@ void reds_client_disconnect(RedClient *client)
}
if (!client || client->disconnecting) {
+ spice_debug("client %p already during disconnection", client);
return;
}
diff --git a/server/reds.h b/server/reds.h
index c5c557d..1c5ae84 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -136,6 +136,8 @@ void reds_handle_agent_mouse_event(const VDAgentMouseState *mouse_state); // use
extern struct SpiceCoreInterface *core;
// Temporary measures to make splitting reds.c to inputs_channel.c easier
+
+/* should be called only from main_dispatcher */
void reds_client_disconnect(RedClient *client);
// Temporary (?) for splitting main channel
--
1.8.3.1

56
0005-reds-s-red_client_disconnect-red_channel_client_shut.patch
View File

@ -1,56 +0,0 @@
From 46c2ce8f1af0170a2c6a335cc743a3ddff2d96d0 Mon Sep 17 00:00:00 2001
From: Yonit Halperin <yhalperi@redhat.com>
Date: Thu, 25 Jul 2013 14:25:24 -0400
Subject: [PATCH 5/8] reds: s/red_client_disconnect/red_channel_client_shutdown
inside callbacks
When we want to disconnect the main channel from a callback, it is
safer to use red_channel_client_shutdown, instead of directly
destroying the client. It is also more consistent with how other
channels treat errors.
red_channel_client_shutdown will trigger socket error in the main channel.
Then, main_channel_client_on_disconnect will be called,
and eventually, main_dispatcher_client_disconnect.
I didn't replace calls to reds_disconnect/reds_client_disconnect in
places where those calls were safe && that might need immediate client
disconnection.
---
server/reds.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index c66ddc4..ae87c90 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -879,7 +879,8 @@ static void vdi_port_on_free_self_token(void *opaque)
static void vdi_port_remove_client(RedClient *client, void *opaque)
{
- reds_client_disconnect(client);
+ red_channel_client_shutdown(main_channel_client_get_base(
+ red_client_get_main(client)));
}
/****************************************************************************/
@@ -1009,7 +1010,7 @@ void reds_on_main_agent_start(MainChannelClient *mcc, uint32_t num_tokens)
if (!client_added) {
spice_warning("failed to add client to agent");
- reds_client_disconnect(rcc->client);
+ red_channel_client_shutdown(rcc);
return;
}
} else {
@@ -1126,7 +1127,7 @@ void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t size)
reds_on_main_agent_monitors_config(mcc, message, size);
return;
case AGENT_MSG_FILTER_PROTO_ERROR:
- reds_disconnect();
+ red_channel_client_shutdown(main_channel_client_get_base(mcc));
return;
}
--
1.8.3.1

42
0006-snd_worker-fix-memory-leak-of-PlaybackChannel.patch
View File

@ -1,42 +0,0 @@
From 134b7f310de5120b233670d18641d32204f31318 Mon Sep 17 00:00:00 2001
From: Yonit Halperin <yhalperi@redhat.com>
Date: Thu, 25 Jul 2013 14:49:33 -0400
Subject: [PATCH 6/8] snd_worker: fix memory leak of PlaybackChannel
When the sequence of calls bellow occurs, the PlaybackChannel
is not released (snd_channel_put is not called for the
samples that refer to the channel).
spice_server_playback_get_buffer
snd_channel_disconnect
spice_server_playback_put_samples
---
server/snd_worker.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/server/snd_worker.c b/server/snd_worker.c
index d6ec47a..849f002 100644
--- a/server/snd_worker.c
+++ b/server/snd_worker.c
@@ -1097,14 +1097,13 @@ SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance
PlaybackChannel *playback_channel;
AudioFrame *frame;
- if (!sin->st->worker.connection) {
- return;
- }
-
frame = SPICE_CONTAINEROF(samples, AudioFrame, samples);
playback_channel = frame->channel;
- if (!snd_channel_put(&playback_channel->base) || !playback_channel->base.worker->connection) {
+ spice_assert(playback_channel);
+ if (!snd_channel_put(&playback_channel->base) ||
+ sin->st->worker.connection != &playback_channel->base) {
/* lost last reference, channel has been destroyed previously */
+ spice_info("audio samples belong to a disconnected channel");
return;
}
spice_assert(playback_channel->base.active);
--
1.8.3.1

62
0007-snd_worker-snd_disconnect_channel-don-t-call-snd_cha.patch
View File

@ -1,62 +0,0 @@
From 02f44c137df99ed2e89699e49b64c13673b0cd06 Mon Sep 17 00:00:00 2001
From: Yonit Halperin <yhalperi@redhat.com>
Date: Thu, 25 Jul 2013 15:07:43 -0400
Subject: [PATCH 7/8] snd_worker/snd_disconnect_channel: don't call
snd_channel_put if the channel has already been disconnected
The snd channels has one reference as long as their socket is active.
The playback channel has an additional reference for each frame that is
currently filled by the sound device.
Once the channel is disconnected (the socket has been freed and the
first reference is released) snd_disconnect_channel shouldn't release
a reference again.
---
server/snd_worker.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/server/snd_worker.c b/server/snd_worker.c
index 849f002..3827416 100644
--- a/server/snd_worker.c
+++ b/server/snd_worker.c
@@ -212,21 +212,20 @@ static void snd_disconnect_channel(SndChannel *channel)
{
SndWorker *worker;
- if (!channel) {
+ if (!channel || !channel->stream) {
spice_debug("not connected");
return;
}
spice_debug("%p", channel);
worker = channel->worker;
- if (channel->stream) {
- channel->cleanup(channel);
- red_channel_client_disconnect(worker->connection->channel_client);
- core->watch_remove(channel->stream->watch);
- channel->stream->watch = NULL;
- reds_stream_free(channel->stream);
- channel->stream = NULL;
- spice_marshaller_destroy(channel->send_data.marshaller);
- }
+ channel->cleanup(channel);
+ red_channel_client_disconnect(worker->connection->channel_client);
+ worker->connection->channel_client = NULL;
+ core->watch_remove(channel->stream->watch);
+ channel->stream->watch = NULL;
+ reds_stream_free(channel->stream);
+ channel->stream = NULL;
+ spice_marshaller_destroy(channel->send_data.marshaller);
snd_channel_put(channel);
worker->connection = NULL;
}
@@ -897,6 +896,7 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i
int tos;
MainChannelClient *mcc = red_client_get_main(client);
+ spice_assert(stream);
if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
spice_printerr("accept failed, %s", strerror(errno));
goto error1;
--
1.8.3.1

99
0008-log-improve-debug-information-related-to-client-disc.patch
View File

@ -1,99 +0,0 @@
From c2e46b926e0ee4226f0f93942e7fa2c5b409f73d Mon Sep 17 00:00:00 2001
From: Yonit Halperin <yhalperi@redhat.com>
Date: Fri, 26 Jul 2013 12:15:00 -0400
Subject: [PATCH 8/8] log: improve debug information related to client
disconnection
---
server/red_channel.c | 9 ++++++---
server/snd_worker.c | 7 ++++---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/server/red_channel.c b/server/red_channel.c
index 9168b8a..d565634 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -1112,6 +1112,7 @@ static void red_channel_client_ref(RedChannelClient *rcc)
static void red_channel_client_unref(RedChannelClient *rcc)
{
if (!--rcc->refs) {
+ spice_debug("destroy rcc=%p", rcc);
if (rcc->send_data.main.marshaller) {
spice_marshaller_destroy(rcc->send_data.main.marshaller);
}
@@ -1708,6 +1709,8 @@ static void red_channel_client_disconnect_dummy(RedChannelClient *rcc)
{
spice_assert(rcc->dummy);
if (ring_item_is_linked(&rcc->channel_link)) {
+ spice_printerr("rcc=%p (channel=%p type=%d id=%d)", rcc, rcc->channel,
+ rcc->channel->type, rcc->channel->id);
red_channel_remove_client(rcc);
}
rcc->dummy_connected = FALSE;
@@ -1715,8 +1718,6 @@ static void red_channel_client_disconnect_dummy(RedChannelClient *rcc)
void red_channel_client_disconnect(RedChannelClient *rcc)
{
- spice_printerr("%p (channel %p type %d id %d)", rcc, rcc->channel,
- rcc->channel->type, rcc->channel->id);
if (rcc->dummy) {
red_channel_client_disconnect_dummy(rcc);
return;
@@ -1724,6 +1725,8 @@ void red_channel_client_disconnect(RedChannelClient *rcc)
if (!red_channel_client_is_connected(rcc)) {
return;
}
+ spice_printerr("rcc=%p (channel=%p type=%d id=%d)", rcc, rcc->channel,
+ rcc->channel->type, rcc->channel->id);
red_channel_client_pipe_clear(rcc);
if (rcc->stream->watch) {
rcc->channel->core->watch_remove(rcc->stream->watch);
@@ -2007,7 +2010,7 @@ void red_client_destroy(RedClient *client)
RingItem *link, *next;
RedChannelClient *rcc;
- spice_printerr("destroy client with #channels %d", client->channels_num);
+ spice_printerr("destroy client %p with #channels=%d", client, client->channels_num);
if (!pthread_equal(pthread_self(), client->thread_id)) {
spice_warning("client->thread_id (0x%lx) != pthread_self (0x%lx)."
"If one of the threads is != io-thread && != vcpu-thread,"
diff --git a/server/snd_worker.c b/server/snd_worker.c
index 3827416..ebddfcd 100644
--- a/server/snd_worker.c
+++ b/server/snd_worker.c
@@ -201,8 +201,8 @@ static SndChannel *snd_channel_get(SndChannel *channel)
static SndChannel *snd_channel_put(SndChannel *channel)
{
if (!--channel->refs) {
+ spice_printerr("SndChannel=%p freed", channel);
free(channel);
- spice_printerr("sound channel freed");
return NULL;
}
return channel;
@@ -216,7 +216,8 @@ static void snd_disconnect_channel(SndChannel *channel)
spice_debug("not connected");
return;
}
- spice_debug("%p", channel);
+ spice_debug("SndChannel=%p rcc=%p type=%d",
+ channel, channel->channel_client, channel->channel_client->channel->type);
worker = channel->worker;
channel->cleanup(channel);
red_channel_client_disconnect(worker->connection->channel_client);
@@ -976,11 +977,11 @@ static void snd_disconnect_channel_client(RedChannelClient *rcc)
{
SndWorker *worker;
- spice_debug(NULL);
spice_assert(rcc->channel);
spice_assert(rcc->channel->data);
worker = (SndWorker *)rcc->channel->data;
+ spice_debug("channel-type=%d", rcc->channel->type);
if (worker->connection) {
spice_assert(worker->connection->channel_client == rcc);
snd_disconnect_channel(worker->connection);
--
1.8.3.1

66
0009-red_worker-decrease-the-timeout-when-flushing-comman.patch
View File

@ -1,66 +0,0 @@
From 6ced0f698507de02a67cfcd25b7ab07e8c423fad Mon Sep 17 00:00:00 2001
Message-Id: <6ced0f698507de02a67cfcd25b7ab07e8c423fad.1377595745.git.uril@redhat.com>
From: Yonit Halperin <yhalperi@redhat.com>
Date: Mon, 5 Aug 2013 12:10:15 -0400
Subject: [PATCH] red_worker: decrease the timeout when flushing commands and waiting for the client.
150 seconds is way too long period for holding the guest driver and
waiting for a response for the client. This timeout was 15 seconds, but
when off-screen surfaces ware introduced it was arbitrarily multiplied by
10.
Other existing related bugs emphasize why it is important to decrease
the timeout:
(1) 994211 - the qxl driver waits for an async-io reponse for 60 seconds
and after that, it switches to sync-io mode. Not only that the
driver might use invalid data (since it didn't wait for the query to
complete), falling back to sync-io mode introduces other errors.
(2) 994175 - spice server sometimes doesn't recognize that the client
has disconnected.
(3) There might be cache inconsistency between the client and the server,
and then the display channel waits indefinitely for a cache item (e.g., bug
977998)
This patch changes the timeout to 30 seconds. I tested it under wifi +emulating 2.5Mbps network,
together with playing video on the guest and changing resolutions in a loop. The timeout didn't expired
during my tests.
This bug is related to rhbz#964136 (but from rhbz#964136 info it is still not
clear why the client wasn't responsive).
---
server/red_worker.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index b45208b..9896696 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -103,7 +103,7 @@
#define CHANNEL_PUSH_TIMEOUT 30000000000ULL //nano
#define CHANNEL_PUSH_SLEEP_DURATION 10000 //micro
-#define DISPLAY_CLIENT_TIMEOUT 15000000000ULL //nano
+#define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano
#define DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT 10000000000ULL //nano, 10 sec
#define DISPLAY_CLIENT_RETRY_INTERVAL 10000 //micro
@@ -9716,7 +9716,7 @@ static inline void flush_display_commands(RedWorker *worker)
if (ring_is_empty) {
break;
}
- end_time = red_now() + DISPLAY_CLIENT_TIMEOUT * 10;
+ end_time = red_now() + DISPLAY_CLIENT_TIMEOUT;
int sleep_count = 0;
for (;;) {
red_channel_push(&worker->display_channel->common.base);
@@ -9760,7 +9760,7 @@ static inline void flush_cursor_commands(RedWorker *worker)
if (ring_is_empty) {
break;
}
- end_time = red_now() + DISPLAY_CLIENT_TIMEOUT * 10;
+ end_time = red_now() + DISPLAY_CLIENT_TIMEOUT;
int sleep_count = 0;
for (;;) {
red_channel_push(&worker->cursor_channel->common.base);
--
1.7.1

2
sources
View File

@ -1 +1 @@
325b1c42ce24e75de45a75876b73a8bd spice-0.12.4.tar.bz2
1256286214fe402703c0a01bd3a85319 spice-0.12.5.tar.bz2

17
spice.spec
View File

@ -1,21 +1,11 @@
Name: spice
Version: 0.12.4
Release: 3%{?dist}
Version: 0.12.5
Release: 1%{?dist}
Summary: Implements the SPICE protocol
Group: User Interface/Desktops
License: LGPLv2+
URL: http://www.spice-space.org/
Source0: http://www.spice-space.org/download/releases/%{name}-%{version}.tar.bz2
Patch1: 0001-red_channel-prevent-adding-and-pushing-pipe-items-af.patch
Patch2: 0002-red_channel-add-ref-count-to-RedClient.patch
Patch3: 0003-main_dispatcher-add-ref-count-protection-to-RedClien.patch
Patch4: 0004-decouple-disconnection-of-the-main-channel-from-clie.patch
Patch5: 0005-reds-s-red_client_disconnect-red_channel_client_shut.patch
Patch6: 0006-snd_worker-fix-memory-leak-of-PlaybackChannel.patch
Patch7: 0007-snd_worker-snd_disconnect_channel-don-t-call-snd_cha.patch
Patch8: 0008-log-improve-debug-information-related-to-client-disc.patch
Patch9: 0009-red_worker-decrease-the-timeout-when-flushing-comman.patch
Patch10: 0010-Fix-buffer-overflow-when-decrypting-client-SPICE-tic.patch
# https://bugzilla.redhat.com/show_bug.cgi?id=613529
%if 0%{?rhel}
@ -110,6 +100,9 @@ mkdir -p %{buildroot}%{_libexecdir}
%changelog
* Mon May 19 2014 Christophe Fergeau <cfergeau@redhat.com> 0.12.5-1
- Update to new 0.12.5 release
* Wed Oct 30 2013 Christophe Fergeau <cfergeau@redhat.com> 0.12.4-3
- Add patch fixing CVE-2013-4282