[Spice-commits] 2 commits - server/red-channel-client.c
Frediano Ziglio
fziglio at kemper.freedesktop.org
Wed Nov 29 16:57:41 UTC 2017
server/red-channel-client.c | 44 ++++++++++++++++----------------------------
1 file changed, 16 insertions(+), 28 deletions(-)
New commits:
commit 35e9a9d435af9c67ffbedd244f7d38b7b3956d02
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Mon Sep 11 10:14:23 2017 +0100
red-channel-client: Simplify red_channel_client_wait_pipe_item_sent loop
Avoid repeating the same code twice.
red_channel_client_send sends the pending item (or a part of it). If
there are no item pending, the function does nothing (so checking for
blocked channel is useless). Also red_channel_client_send is already
called from red_channel_client_push which has a check for blocked
channels, so having calls to both red_channel_client_send() and
red_channel_client_push() is redundant.
The function on its overall tries to wait for a given item to be sent.
The call for red_channel_client_receive is mainly needed to support the
cases were to send data messages from the client should be processed
(like if "handle-acks" is requested).
Moving the loop iteration check inside the for loop instead allows to
avoid some duplication.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Christophe Fergeau <cfergeau at redhat.com>
diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 7ca65619..086c5fa4 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -1776,18 +1776,14 @@ bool red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,
red_pipe_item_ref(&mark_item->base);
red_channel_client_pipe_add_after_pos(rcc, &mark_item->base, item_pos);
- if (red_channel_client_is_blocked(rcc)) {
- red_channel_client_receive(rcc);
- red_channel_client_send(rcc);
- }
- red_channel_client_push(rcc);
-
- while (mark_item->item_in_pipe &&
- (timeout == -1 || spice_get_monotonic_time_ns() < end_time)) {
- usleep(CHANNEL_BLOCKED_SLEEP_DURATION);
+ for (;;) {
red_channel_client_receive(rcc);
- red_channel_client_send(rcc);
red_channel_client_push(rcc);
+ if (!mark_item->item_in_pipe ||
+ (timeout != -1 && spice_get_monotonic_time_ns() >= end_time)) {
+ break;
+ }
+ usleep(CHANNEL_BLOCKED_SLEEP_DURATION);
}
item_in_pipe = mark_item->item_in_pipe;
commit b2fa6ec66c54cb8cae25ef9a7c4e78fa49de453b
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Mon Sep 11 10:07:56 2017 +0100
red-channel-client: Avoid weird memory references using MarkerPipeItem
Instead of having MarkerPipeItem pointing to an external variable with
the possibility to forget to reset it and have a dangling pointer, this
commit takes a reference on the item to keep it alive after it was sent.
This item is placed into the queue to understand when it was sent. The
current implementation detects the unqueue when the item is destroyed so
we currently store a pointer to an external variable in the item, this
way we can use a variable which will still be alive after the item is
released/destroyed.
This change updates the variable (stored in the item) when we try to
send the item, rather than at destruction time. The destruction happened
at the end of red_channel_client_send_item(), so we don't mark
item_in_pipe much earlier than before.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Christophe Fergeau <cfergeau at redhat.com>
diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 821dd351..7ca65619 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -215,7 +215,7 @@ typedef struct RedEmptyMsgPipeItem {
typedef struct MarkerPipeItem {
RedPipeItem base;
- gboolean *item_in_pipe;
+ bool item_in_pipe;
} MarkerPipeItem;
static void red_channel_client_start_ping_timer(RedChannelClient *rcc, uint32_t timeout)
@@ -610,6 +610,7 @@ static void red_channel_client_send_item(RedChannelClient *rcc, RedPipeItem *ite
red_channel_client_send_ping(rcc);
break;
case RED_PIPE_ITEM_TYPE_MARKER:
+ SPICE_UPCAST(MarkerPipeItem, item)->item_in_pipe = false;
break;
default:
red_channel_send_item(rcc->priv->channel, rcc, item);
@@ -1752,23 +1753,13 @@ void red_channel_client_set_header_sub_list(RedChannelClient *rcc, uint32_t sub_
rcc->priv->send_data.header.set_msg_sub_list(&rcc->priv->send_data.header, sub_list);
}
-static void marker_pipe_item_free(RedPipeItem *base)
-{
- MarkerPipeItem *item = SPICE_UPCAST(MarkerPipeItem, base);
-
- if (item->item_in_pipe) {
- *item->item_in_pipe = FALSE;
- }
- g_free(item);
-}
-
/* TODO: more evil sync stuff. anything with the word wait in it's name. */
bool red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,
GList *item_pos,
int64_t timeout)
{
uint64_t end_time;
- gboolean item_in_pipe;
+ bool item_in_pipe;
spice_debug("trace");
@@ -1780,10 +1771,9 @@ bool red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,
MarkerPipeItem *mark_item = g_new0(MarkerPipeItem, 1);
- red_pipe_item_init_full(&mark_item->base, RED_PIPE_ITEM_TYPE_MARKER,
- marker_pipe_item_free);
- item_in_pipe = TRUE;
- mark_item->item_in_pipe = &item_in_pipe;
+ red_pipe_item_init(&mark_item->base, RED_PIPE_ITEM_TYPE_MARKER);
+ mark_item->item_in_pipe = true;
+ red_pipe_item_ref(&mark_item->base);
red_channel_client_pipe_add_after_pos(rcc, &mark_item->base, item_pos);
if (red_channel_client_is_blocked(rcc)) {
@@ -1792,7 +1782,7 @@ bool red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,
}
red_channel_client_push(rcc);
- while (item_in_pipe &&
+ while (mark_item->item_in_pipe &&
(timeout == -1 || spice_get_monotonic_time_ns() < end_time)) {
usleep(CHANNEL_BLOCKED_SLEEP_DURATION);
red_channel_client_receive(rcc);
@@ -1800,9 +1790,11 @@ bool red_channel_client_wait_pipe_item_sent(RedChannelClient *rcc,
red_channel_client_push(rcc);
}
+ item_in_pipe = mark_item->item_in_pipe;
+ red_pipe_item_unref(&mark_item->base);
+
if (item_in_pipe) {
- // still on the queue, make sure won't overwrite the stack variable
- mark_item->item_in_pipe = NULL;
+ // still on the queue
spice_warning("timeout");
return FALSE;
} else {
More information about the Spice-commits
mailing list