On Wed, 2016-11-02 at 16:25 -0500, Jonathon Jongsma wrote:
> Store the channel in the char device object, and unref it in
> dispose.
> Previously, we were just creating a channel and potentially allowing
> it
> to leak. There may be better long-term solutions, but this works
> for
> now.
Acked-by: Pavel Grunt <pgrunt@xxxxxxxxxx>
> ---
> Changes from last version:
> - set channel to NULL in spicevmc_device_disconnect() to avoid the
> double-free
> mentioned by Frediano
>
> server/spicevmc.c | 46 ++++++++++++++++++++++++++++++------------
> ----
> 1 file changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 7067bc7..8e23248 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -47,6 +47,9 @@
> #define BUF_SIZE (64 * 1024 + 32)
> #define COMPRESS_THRESHOLD 1000
>
> +typedef struct RedVmcChannel RedVmcChannel;
> +typedef struct RedVmcChannelClass RedVmcChannelClass;
> +
> typedef struct RedVmcPipeItem {
> RedPipeItem base;
>
> @@ -75,6 +78,7 @@ typedef struct RedCharDeviceSpiceVmcClass
> RedCharDeviceSpiceVmcClass;
>
> struct RedCharDeviceSpiceVmc {
> RedCharDevice parent;
> + RedVmcChannel *channel;
> };
>
> struct RedCharDeviceSpiceVmcClass
> @@ -89,9 +93,6 @@ static RedCharDevice
> *red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
>
> G_DEFINE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc,
> RED_TYPE_CHAR_DEVICE)
>
> -#define RED_CHAR_DEVICE_SPICEVMC_PRIVATE(o) \
> - (G_TYPE_INSTANCE_GET_PRIVATE ((o),
> RED_TYPE_CHAR_DEVICE_SPICEVMC, RedCharDeviceSpiceVmcPrivate))
> -
> #define RED_TYPE_VMC_CHANNEL red_vmc_channel_get_type()
>
> #define RED_VMC_CHANNEL(obj) \
> @@ -103,9 +104,6 @@ G_DEFINE_TYPE(RedCharDeviceSpiceVmc,
> red_char_device_spicevmc, RED_TYPE_CHAR_DEV
> #define RED_VMC_CHANNEL_GET_CLASS(obj) \
> (G_TYPE_INSTANCE_GET_CLASS((obj), RED_TYPE_VMC_CHANNEL,
> RedVmcChannelClass))
>
> -typedef struct RedVmcChannel RedVmcChannel;
> -typedef struct RedVmcChannelClass RedVmcChannelClass;
> -
> struct RedVmcChannel
> {
> RedChannel parent;
> @@ -855,28 +853,33 @@ RedCharDevice
> *spicevmc_device_connect(RedsState *reds,
> SpiceCharDeviceInstance
> *sin,
> uint8_t channel_type)
> {
> + RedCharDeviceSpiceVmc *dev_state;
> RedVmcChannel *state = red_vmc_channel_new(reds, channel_type,
> sin);
>
> - return state->chardev;
> + dev_state = RED_CHAR_DEVICE_SPICEVMC(state->chardev);
> + dev_state->channel = state;
> +
> + return RED_CHAR_DEVICE(dev_state);
> }
>
> /* Must be called from RedClient handling thread. */
> void spicevmc_device_disconnect(RedsState *reds,
> SpiceCharDeviceInstance *sin)
> {
> - RedVmcChannel *state;
> + RedVmcChannel *channel;
> + RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(sin->st);
>
> - /* FIXME */
> - state = (RedVmcChannel
> *)red_char_device_opaque_get((RedCharDevice*)sin->st);
> + channel = vmc->channel;
> + vmc->channel = NULL;
>
> - red_char_device_write_buffer_release(state->chardev, &state-
> >recv_from_client_buf);
> + red_char_device_write_buffer_release(channel->chardev,
> &channel->recv_from_client_buf);
> /* FIXME */
> - red_char_device_destroy((RedCharDevice*)sin->st);
> - state->chardev = NULL;
> + red_char_device_destroy(RED_CHAR_DEVICE(vmc));
> + channel->chardev = NULL;
> sin->st = NULL;
>
> - reds_unregister_channel(reds, RED_CHANNEL(state));
> - free(state->pipe_item);
> - red_channel_destroy(RED_CHANNEL(state));
> + reds_unregister_channel(reds, RED_CHANNEL(channel));
> + free(channel->pipe_item);
> + red_channel_destroy(RED_CHANNEL(channel));
> }
>
> SPICE_GNUC_VISIBLE void
> spice_server_port_event(SpiceCharDeviceInstance *sin, uint8_t event)
> @@ -904,10 +907,21 @@ SPICE_GNUC_VISIBLE void
> spice_server_port_event(SpiceCharDeviceInstance *sin, ui
> }
>
> static void
> +red_char_device_spicevmc_dispose(GObject *object)
> +{
> + RedCharDeviceSpiceVmc *self = RED_CHAR_DEVICE_SPICEVMC(object);
> +
> + g_clear_object(&self->channel);
> +}
> +
> +static void
> red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass
> *klass)
> {
> + GObjectClass *object_class = G_OBJECT_CLASS(klass);
> RedCharDeviceClass *char_dev_class =
> RED_CHAR_DEVICE_CLASS(klass);
>
> + object_class->dispose = red_char_device_spicevmc_dispose;
> +
> char_dev_class->read_one_msg_from_device =
> spicevmc_chardev_read_msg_from_dev;
> char_dev_class->send_msg_to_client =
> spicevmc_chardev_send_msg_to_client;
> char_dev_class->send_tokens_to_client =
> spicevmc_char_dev_send_tokens_to_client;
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel