resolves: rhbz#891326
Starting from commit 81fe00b08ad4f, red_detach_streams_behind can
trigger modifications in the current tree (by update_area calls). Thus,
after calling red_detach_streams_behind it is not safe to access tree
entries that were calculated before the call.
This patch inserts the drawable to the tree before the call to
red_detach_streams_behind. This change also requires making sure
that rendering operations that can be triggered by
red_detach_streams_behind will not include this drawable (which is now part of the tree).
---
server/red_worker.c | 55 ++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 42 insertions(+), 13 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 1a9c375..5e00cb6 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1022,6 +1022,8 @@ static void red_current_flush(RedWorker *worker, int surface_id);
#else
static void red_draw_drawable(RedWorker *worker, Drawable *item);
static void red_update_area(RedWorker *worker, const SpiceRect *area, int surface_id);
+static void red_update_area_till(RedWorker *worker, const SpiceRect *area, int surface_id,
+ Drawable *last);
#endif
static void red_release_cursor(RedWorker *worker, CursorItem *cursor);
static inline void release_drawable(RedWorker *worker, Drawable *item);
@@ -2615,7 +2617,9 @@ static int red_display_drawable_is_in_pipe(DisplayChannelClient *dcc, Drawable *
* after red_display_detach_stream_gracefully is called for all the display channel clients,
* red_detach_stream should be called. See comment (1).
*/
-static inline void red_display_detach_stream_gracefully(DisplayChannelClient *dcc, Stream *stream)
+static inline void red_display_detach_stream_gracefully(DisplayChannelClient *dcc,
+ Stream *stream,
+ Drawable *update_area_limit)
{
int stream_id = get_stream_id(dcc->common.worker, stream);
StreamAgent *agent = &dcc->stream_agents[stream_id];
@@ -2669,27 +2673,41 @@ static inline void red_display_detach_stream_gracefully(DisplayChannelClient *dc
spice_debug("stream %d: upgrade by screenshot. has current %d. box ==>",
stream_id, stream->current != NULL);
rect_debug(&upgrade_area);
- red_update_area(dcc->common.worker, &upgrade_area, 0);
+ if (update_area_limit) {
+ red_update_area_till(dcc->common.worker, &upgrade_area, 0, update_area_limit);
+ } else {
+ red_update_area(dcc->common.worker, &upgrade_area, 0);
+ }
red_add_surface_area_image(dcc, 0, &upgrade_area, NULL, FALSE);
}
}
-static inline void red_detach_stream_gracefully(RedWorker *worker, Stream *stream)
+static inline void red_detach_stream_gracefully(RedWorker *worker, Stream *stream,
+ Drawable *update_area_limit)
{
RingItem *item;
DisplayChannelClient *dcc;
WORKER_FOREACH_DCC(worker, item, dcc) {
- red_display_detach_stream_gracefully(dcc, stream);
+ red_display_detach_stream_gracefully(dcc, stream, update_area_limit);
}
if (stream->current) {
red_detach_stream(worker, stream, TRUE);
}
}
-// region should be a primary surface region
-static void red_detach_streams_behind(RedWorker *worker, QRegion *region)
+/*
+ * region : a primary surface region. Streams that intersects with the given
+ * region will be detached.
+ * drawable: If detaching the stream is triggered by the addition of a new drawable
+ * that is dependent on the given region, and the drawable is already a part
+ * of the "current tree", the drawable parameter should be set with
+ * this drawable, otherwise, it should be NULL. Then, if detaching the stream
+ * involves sending an upgrade image to the client, this drawable won't be rendered
+ * (see red_display_detach_stream_gracefully).
+ */
+static void red_detach_streams_behind(RedWorker *worker, QRegion *region, Drawable *drawable)
{
Ring *ring = &worker->streams;
RingItem *item = ring_get_head(ring);
@@ -2706,7 +2724,7 @@ static void red_detach_streams_behind(RedWorker *worker, QRegion *region)
StreamAgent *agent = &dcc->stream_agents[get_stream_id(worker, stream)];
if (region_intersects(&agent->vis_region, region)) {
- red_display_detach_stream_gracefully(dcc, stream);
+ red_display_detach_stream_gracefully(dcc, stream, drawable);
detach_stream = 1;
spice_debug("stream %d", get_stream_id(worker, stream));
}
@@ -2798,7 +2816,7 @@ static inline void red_handle_streams_timout(RedWorker *worker)
Stream *stream = SPICE_CONTAINEROF(item, Stream, link);
item = ring_next(ring, item);
if (now >= (stream->last_time + RED_STREAM_TIMOUT)) {
- red_detach_stream_gracefully(worker, stream);
+ red_detach_stream_gracefully(worker, stream, NULL);
red_stop_stream(worker, stream);
}
}
@@ -3507,13 +3525,24 @@ static inline int red_current_add(RedWorker *worker, Ring *ring, Drawable *drawa
exclude_region(worker, ring, exclude_base, &exclude_rgn, NULL, drawable);
red_use_stream_trace(worker, drawable);
red_streams_update_visible_region(worker, drawable);
+ /*
+ * Performing the insertion after exclude_region for
+ * safety (todo: Not sure if exclude_region can affect the drawable
+ * if it is added to the tree before calling exclude_region).
+ */
+ __current_add_drawable(worker, drawable, ring);
} else {
+ /*
+ * red_detach_streams_behind can affect the current tree since it may
+ * trigger calls to update_area. Thus, the drawable should be added to the tree
+ * before calling red_detach_streams_behind
+ */
+ __current_add_drawable(worker, drawable, ring);
if (drawable->surface_id == 0) {
- red_detach_streams_behind(worker, &drawable->tree_item.base.rgn);
+ red_detach_streams_behind(worker, &drawable->tree_item.base.rgn, drawable);
}
}
region_destroy(&exclude_rgn);
- __current_add_drawable(worker, drawable, ring);
stat_add(&worker->add_stat, start_time);
return TRUE;
}
@@ -3567,7 +3596,7 @@ static inline int red_current_add_with_shadow(RedWorker *worker, Ring *ring, Dra
// only primary surface streams are supported
if (is_primary_surface(worker, item->surface_id)) {
- red_detach_streams_behind(worker, &shadow->base.rgn);
+ red_detach_streams_behind(worker, &shadow->base.rgn, NULL);
}
ring_add(ring, &shadow->base.siblings_link);
__current_add_drawable(worker, item, ring);
@@ -3579,7 +3608,7 @@ static inline int red_current_add_with_shadow(RedWorker *worker, Ring *ring, Dra
red_streams_update_visible_region(worker, item);
} else {
if (item->surface_id == 0) {
- red_detach_streams_behind(worker, &item->tree_item.base.rgn);
+ red_detach_streams_behind(worker, &item->tree_item.base.rgn, item);
}
}
stat_add(&worker->add_stat, start_time);
@@ -3911,7 +3940,7 @@ static inline int red_handle_surfaces_dependencies(RedWorker *worker, Drawable *
QRegion depend_region;
region_init(&depend_region);
region_add(&depend_region, &drawable->red_drawable->surfaces_rects[x]);
- red_detach_streams_behind(worker, &depend_region);
+ red_detach_streams_behind(worker, &depend_region, NULL);
}
}
}