New upstream bug-fix release 0.12.4
- Add patches from upstream git to fix sound-channel-free crash (rhbz#986407) - Add Obsoletes for dropped spice-client sub-package - New upstream bug-fix release 0.12.4 - Add patches from upstream git to fix sound-channel-free crash (rhbz#986407) - Add Obsoletes for dropped spice-client sub-package
This commit is contained in:
parent
10cbec62a1
commit
1081338830
1
.gitignore
vendored
1
.gitignore
vendored
@ -16,3 +16,4 @@ spice-0.5.3.tar.bz2
|
||||
/spice-0.12.0.tar.bz2
|
||||
/spice-0.12.2.tar.bz2
|
||||
/spice-0.12.3.tar.bz2
|
||||
/spice-0.12.4.tar.bz2
|
||||
|
@ -0,0 +1,96 @@
|
||||
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
Normal file
96
0002-red_channel-add-ref-count-to-RedClient.patch
Normal file
@ -0,0 +1,96 @@
|
||||
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
|
||||
|
@ -0,0 +1,51 @@
|
||||
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
Normal file
238
0004-decouple-disconnection-of-the-main-channel-from-clie.patch
Normal file
@ -0,0 +1,238 @@
|
||||
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
|
||||
|
@ -0,0 +1,56 @@
|
||||
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
Normal file
42
0006-snd_worker-fix-memory-leak-of-PlaybackChannel.patch
Normal file
@ -0,0 +1,42 @@
|
||||
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
|
||||
|
@ -0,0 +1,62 @@
|
||||
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
|
||||
|
@ -0,0 +1,99 @@
|
||||
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
|
||||
|
2
sources
2
sources
@ -1 +1 @@
|
||||
f33a682892f6793169f20298b2296449 spice-0.12.3.tar.bz2
|
||||
325b1c42ce24e75de45a75876b73a8bd spice-0.12.4.tar.bz2
|
||||
|
31
spice.spec
31
spice.spec
@ -1,11 +1,19 @@
|
||||
Name: spice
|
||||
Version: 0.12.3
|
||||
Release: 2%{?dist}
|
||||
Version: 0.12.4
|
||||
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
|
||||
|
||||
# https://bugzilla.redhat.com/show_bug.cgi?id=613529
|
||||
%if 0%{?rhel}
|
||||
@ -33,6 +41,7 @@ variety of machine architectures.
|
||||
%package server
|
||||
Summary: Implements the server side of the SPICE protocol
|
||||
Group: System Environment/Libraries
|
||||
Obsoletes: spice-client < %{version}-%{release}
|
||||
|
||||
%description server
|
||||
The Simple Protocol for Independent Computing Environments (SPICE) is
|
||||
@ -48,7 +57,7 @@ to be a SPICE server.
|
||||
%package server-devel
|
||||
Summary: Header files, libraries and development documentation for spice-server
|
||||
Group: Development/Libraries
|
||||
Requires: %{name}-server = %{version}-%{release}
|
||||
Requires: %{name}-server%{?_isa} = %{version}-%{release}
|
||||
Requires: pkgconfig
|
||||
|
||||
%description server-devel
|
||||
@ -59,6 +68,14 @@ using spice-server, you will need to install spice-server-devel.
|
||||
|
||||
%prep
|
||||
%setup -q
|
||||
%patch1 -p1
|
||||
%patch2 -p1
|
||||
%patch3 -p1
|
||||
%patch4 -p1
|
||||
%patch5 -p1
|
||||
%patch6 -p1
|
||||
%patch7 -p1
|
||||
%patch8 -p1
|
||||
|
||||
|
||||
%build
|
||||
@ -75,8 +92,6 @@ mkdir -p %{buildroot}%{_libexecdir}
|
||||
|
||||
|
||||
%post server -p /sbin/ldconfig
|
||||
|
||||
|
||||
%postun server -p /sbin/ldconfig
|
||||
|
||||
|
||||
@ -84,7 +99,6 @@ mkdir -p %{buildroot}%{_libexecdir}
|
||||
%doc COPYING README NEWS
|
||||
%{_libdir}/libspice-server.so.1*
|
||||
|
||||
|
||||
%files server-devel
|
||||
%{_includedir}/spice-server
|
||||
%{_libdir}/libspice-server.so
|
||||
@ -92,6 +106,11 @@ mkdir -p %{buildroot}%{_libexecdir}
|
||||
|
||||
|
||||
%changelog
|
||||
* Fri Aug 2 2013 Hans de Goede <hdegoede@redhat.com> - 0.12.4-1
|
||||
- New upstream bug-fix release 0.12.4
|
||||
- Add patches from upstream git to fix sound-channel-free crash (rhbz#986407)
|
||||
- Add Obsoletes for dropped spice-client sub-package
|
||||
|
||||
* Thu May 23 2013 Christophe Fergeau <cfergeau@redhat.com> 0.12.3-2
|
||||
- Stop building spicec, it's obsolete and superseded by remote-viewer
|
||||
(part of virt-viewer)
|
||||
|
Loading…
Reference in New Issue
Block a user