[Spice-devel] [PATCH spice-gtk] widget: fix invalid memory ref after channel is distroyed
Hans de Goede
hdegoede at redhat.com
Sat Mar 31 02:27:19 PDT 2012
Ack.
On 03/31/2012 01:39 AM, Marc-André Lureau wrote:
> When the display channel is destroyed, we disconnect all signals
> handlers, but we don't remove the reference on the primary surface
> data, and that can lead to crashes in a later expose event, reusing
> the canvas surface (ex, if scaling is disabled). Call
> primary_destroy() when disconnecting the channel from the widget.
>
> We now keep the primary surface during channel reset (right after
> disconnect for example), so the primary surface can be eventually
> recycled, and the widget still holds a valid reference until the
> signal is received. The primary surface is ultimately destroyed during
> finalize, or if the new primary surface size doesn't match.
>
> Program received signal SIGSEGV, Segmentation fault.
> __memmove_ssse3_back () at ../sysdeps/x86_64/multiarch/memcpy-ssse3-back.S:2130
> 2130 lddqu -68(%rsi), %xmm0
> Missing separate debuginfos, use: debuginfo-install gtk2-engines-2.20.2-2.fc15.x86_64 libusb1-1.0.9-0.3.rc1.fc16.x86_64 p11-kit-0.6-1.fc16.x86_64
> (gdb) bt
> at ../sysdeps/x86_64/multiarch/memcpy-ssse3-back.S:2130
> srclen=<optimized out>, srcinc=4096, destinc=68, height=<optimized out>,
> half_order=0) at /usr/include/bits/string3.h:52
> dest_bits_per_pixel=32, req_yoffset=<optimized out>, req_xoffset=0,
> image=0x7fffffffb9a0, req=<optimized out>, dpy=0x64a630) at PutImage.c:821
> req_height=<optimized out>, req_width=<optimized out>, y=<optimized out>,
> x=0, req_yoffset=<optimized out>, req_xoffset=0, image=0x7fffffffb9a0,
> gc=0xa817e0, d=33554452, dpy=0x64a630) at PutImage.c:870
> req_xoffset=0, req_yoffset=<optimized out>, x=0, y=26, req_width=17,
> req_height=20, dest_bits_per_pixel=32, dest_scanline_pad=32)
> at PutImage.c:908
> image=0x7fffffffb9a0, req_xoffset=0, req_yoffset=0, x=0, y=26,
> req_width=17, req_height=20) at PutImage.c:1027
> image=<optimized out>, src_x=0, src_y=0, width=17, height=20, dst_x=0,
> dst_y=26) at cairo-xlib-surface.c:1357
> ---Type<return> to continue, or q<return> to quit---c
> height=20, width=17, dst_y=26, dst_x=0, src_y=<optimized out>,
> src_x=<optimized out>, pattern=0x7fffffffc6b0, op=CAIRO_OPERATOR_OVER,
> surface=0xb9a650) at cairo-xlib-surface.c:2403
> dst_y=26, dst_x=0, mask_y=0, mask_x=0, src_y=26, src_x=0,
> abstract_dst=0xb9a650, mask_pattern=0x0, src_pattern=0x7fffffffc6b0,
> op=CAIRO_OPERATOR_OVER) at cairo-xlib-surface.c:2452
> src_pattern=0x7fffffffc6b0, mask_pattern=0x0, abstract_dst=0xb9a650,
> src_x=0, src_y=26, mask_x=0, mask_y=0, dst_x=0, dst_y=26, width=17,
> height=20, clip_region=0x0) at cairo-xlib-surface.c:2415
> src=0x7fffffffc6b0, mask=0x0, dst=0xb9a650, src_x=0, src_y=26, mask_x=0,
> mask_y=0, dst_x=0, dst_y=26, width=17, height=20, clip_region=0x0)
> at cairo-surface.c:1802
> traps=0x7fffffffbee0, src=0x7fffffffc6b0, op=CAIRO_OPERATOR_OVER,
> dst=0xb9a650) at cairo-surface-fallback.c:762
> op=CAIRO_OPERATOR_OVER, dst=0xb9a650, traps=0x7fffffffbee0,
> antialias=CAIRO_ANTIALIAS_DEFAULT, clip=0x0, extents=0x7fffffffc600)
> at cairo-surface-fallback.c:812
> ---Type<return> to continue, or q<return> to quit---bt
> op=CAIRO_OPERATOR_OVER, source=0x7fffffffc6b0, clip=0x0)
> at cairo-surface-fallback.c:935
> source=0x7fffffffc6b0, op=CAIRO_OPERATOR_OVER, surface=0xb9a650)
> at cairo-surface.c:2027
> source=0x7fffffffc6b0, clip=0x7fffffffc7b0) at cairo-surface.c:1993
> at cairo-gstate.c:1049
> at spice-widget-cairo.c:104
> expose=0x7fffffffceb0) at spice-widget-cairo.c:133
> expose=0x7fffffffceb0) at spice-widget.c:885
> return_value=0x7fffffffca60, n_param_values=<optimized out>,
> param_values=0x7fffffffcad0, invocation_hint=<optimized out>,
> marshal_data=<optimized out>) at gtkmarshalers.c:86
> return_value=0x7fffffffca60, n_param_values=2,
> param_values=0x7fffffffcad0, invocation_hint=<optimized out>)
> ---Type<return> to continue, or q<return> to quit---c
> at gclosure.c:777
> detail=0, instance=<optimized out>, emission_return=0x7fffffffccb0,
> instance_and_params=0x7fffffffcad0) at gsignal.c:3584
> signal_id=<optimized out>, detail=0, var_args=<optimized out>)
> at gsignal.c:3305
> signal_id=<optimized out>, detail=<optimized out>) at gsignal.c:3351
> event=0x7fffffffceb0) at gtkwidget.c:4999
> ---
> gtk/channel-display.c | 27 +++++++--------------------
> gtk/spice-widget.c | 1 +
> 2 files changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/gtk/channel-display.c b/gtk/channel-display.c
> index 8269705..d3109bc 100644
> --- a/gtk/channel-display.c
> +++ b/gtk/channel-display.c
> @@ -73,7 +73,6 @@ struct _SpiceDisplayChannelPrivate {
> #ifdef WIN32
> HDC dc;
> #endif
> - gboolean migrate_wait_primary;
> };
>
> G_DEFINE_TYPE(SpiceDisplayChannel, spice_display_channel, SPICE_TYPE_CHANNEL)
> @@ -99,7 +98,7 @@ static guint signals[SPICE_DISPLAY_LAST_SIGNAL];
> static void spice_display_handle_msg(SpiceChannel *channel, SpiceMsgIn *msg);
> static void spice_display_channel_up(SpiceChannel *channel);
>
> -static void clear_surfaces(SpiceChannel *channel);
> +static void clear_surfaces(SpiceChannel *channel, gboolean keep_primary);
> static void clear_streams(SpiceChannel *channel);
> static display_surface *find_surface(SpiceDisplayChannelPrivate *c, int surface_id);
> static gboolean display_stream_render(display_stream *st);
> @@ -123,11 +122,7 @@ static void spice_display_channel_dispose(GObject *object)
>
> static void spice_display_channel_finalize(GObject *object)
> {
> - SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(object)->priv;
> -
> - c->migrate_wait_primary = FALSE;
> -
> - clear_surfaces(SPICE_CHANNEL(object));
> + clear_surfaces(SPICE_CHANNEL(object), FALSE);
> clear_streams(SPICE_CHANNEL(object));
>
> if (G_OBJECT_CLASS(spice_display_channel_parent_class)->finalize)
> @@ -190,13 +185,9 @@ static void spice_display_set_property(GObject *object,
> /* main or coroutine context */
> static void spice_display_channel_reset(SpiceChannel *channel, gboolean migrating)
> {
> - SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
> -
> /* palettes, images, and glz_window are cleared in the session */
> -
> - c->migrate_wait_primary = migrating;
> clear_streams(channel);
> - clear_surfaces(channel);
> + clear_surfaces(channel, TRUE);
>
> SPICE_CHANNEL_CLASS(spice_display_channel_parent_class)->channel_reset(channel, migrating);
> }
> @@ -618,10 +609,8 @@ static int create_canvas(SpiceChannel *channel, display_surface *surface)
> display_surface *primary = find_surface(c, 0);
>
> if (primary) {
> - if (c->migrate_wait_primary&&
> - primary->width == surface->width&&
> + if (primary->width == surface->width&&
> primary->height == surface->height) {
> - c->migrate_wait_primary = FALSE;
> SPICE_DEBUG("Reusing existing primary surface");
> return 0;
> }
> @@ -633,8 +622,6 @@ static int create_canvas(SpiceChannel *channel, display_surface *surface)
> }
>
> SPICE_DEBUG("display: create primary canvas");
> - c->migrate_wait_primary = FALSE;
> -
> #ifdef HAVE_SYS_SHM_H
> surface->shmid = shmget(IPC_PRIVATE, surface->size, IPC_CREAT | 0777);
> if (surface->shmid>= 0) {
> @@ -730,7 +717,7 @@ static display_surface *find_surface(SpiceDisplayChannelPrivate *c, int surface_
> return NULL;
> }
>
> -static void clear_surfaces(SpiceChannel *channel)
> +static void clear_surfaces(SpiceChannel *channel, gboolean keep_primary)
> {
> SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
> display_surface *surface;
> @@ -740,8 +727,8 @@ static void clear_surfaces(SpiceChannel *channel)
> surface = SPICE_CONTAINEROF(item, display_surface, link);
> item = ring_next(&c->surfaces, item);
>
> - if (c->migrate_wait_primary&& surface->primary) {
> - SPICE_DEBUG("Try to keep exisiting primary surface during migration");
> + if (keep_primary&& surface->primary) {
> + SPICE_DEBUG("keeping exisiting primary surface, migration or reset");
> continue;
> }
>
> diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
> index a353fda..c4ccc56 100644
> --- a/gtk/spice-widget.c
> +++ b/gtk/spice-widget.c
> @@ -1755,6 +1755,7 @@ static void disconnect_display(SpiceDisplay *display)
> display);
> g_signal_handlers_disconnect_by_func(d->display, G_CALLBACK(invalidate),
> display);
> + primary_destroy(d->display, display);
> d->display = NULL;
> }
>
More information about the Spice-devel
mailing list