Hi,
On Tue, Apr 17, 2018 at 09:45:10AM -0400, Frediano Ziglio wrote:
> >
> > On Tue, Apr 17, 2018 at 01:32:53PM +0100, Frediano Ziglio wrote:
> > > This structure is potentially used in multiple thread.
> > > Currently in Gstreamer thread using streaming data and coroutine
> > > thread.
> >
> > But only GStreamer uses another thread, so this might be
> > suboptimal for all other channels.
> >
> > >
> > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > ---
> > > I would avoid the atomic penalty but better safe then sorry
>
> Yes, that is my comment too... do you have another safe solution?
>
> I was thinking about adding an atomic reference counting for
> SpiceFrame instead and using it for instance, but I'm not sure
> it fixes all the cases.
That was what I was thinking too...
> It would need to remove ref_data/unref_data and have ownership
> from SpiceFrame to "data" (raw message basically) but this
> would mean that refcount would be 2 in queue_frame and
> decremented by coroutine thread and lately by GStreamer thread
> which in theory is racy too.
Important is that last ref/free to happen from the main thread
which should happen but I need to double check the code..
> OT: not much familiar with the threading model of spice-gtk.
> I supposed (beside GStreamer) there were a main thread and a
> coroutine thread. Is the main thread the same as coroutine one?
Yes. Same thread, just context might change (being in one of the
coroutine contexts or not).
toso
>
> > > ---
> > > src/spice-channel-priv.h | 2 +-
> > > src/spice-channel.c | 5 ++---
> > > 2 files changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/spice-channel-priv.h b/src/spice-channel-priv.h
> > > index b431037..706df9f 100644
> > > --- a/src/spice-channel-priv.h
> > > +++ b/src/spice-channel-priv.h
> > > @@ -55,7 +55,7 @@ struct _SpiceMsgOut {
> > > };
> > >
> > > struct _SpiceMsgIn {
> > > - int refcount;
> > > + gint refcount;
> > > SpiceChannel *channel;
> > > uint8_t header[MAX_SPICE_DATA_HEADER_SIZE];
> > > uint8_t *data;
> > > diff --git a/src/spice-channel.c b/src/spice-channel.c
> > > index 7e3e3b7..49897b7 100644
> > > --- a/src/spice-channel.c
> > > +++ b/src/spice-channel.c
> > > @@ -531,7 +531,7 @@ void spice_msg_in_ref(SpiceMsgIn *in)
> > > {
> > > g_return_if_fail(in != NULL);
> > >
> > > - in->refcount++;
> > > + g_atomic_int_inc(&in->refcount);
> > > }
> > >
> > > G_GNUC_INTERNAL
> > > @@ -539,8 +539,7 @@ void spice_msg_in_unref(SpiceMsgIn *in)
> > > {
> > > g_return_if_fail(in != NULL);
> > >
> > > - in->refcount--;
> > > - if (in->refcount > 0)
> > > + if (!g_atomic_int_dec_and_test(&in->refcount))
> > > return;
> > > if (in->parsed)
> > > in->pfree(in->parsed);
>
> Frediano
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel