[Spice-devel] [spice-server] reds: Call sasl_server_done() when destroying SpiceServer instance

[Frediano Ziglio]

> 
> > 
> > This call will free most of the memory allocated by sasl_server_init().
> > It's refcounted so should be safe to call from a library.
> > ---
> >  server/reds.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/server/reds.c b/server/reds.c
> > index 85043a88d..e195ce611 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -3682,6 +3682,7 @@ SPICE_GNUC_VISIBLE void
> > spice_server_destroy(SpiceServer *reds)
> >          g_object_unref(reds->main_dispatcher);
> >      }
> >      reds_cleanup_net(reds);
> > +    sasl_server_done();
> >      g_clear_object(&reds->agent_dev);
> >  
> >      // NOTE: don't replace with g_list_free_full as this function that
> >      passed callback
> 
> Would be great if they document this, see
> https://www.cyrusimap.org/sasl/sasl/reference/manpages/library/sasl_server_done.html
> 
> Note that sasl_server_done is called during spice_server_destroy while
> sasl_server_done is called during spice_server_init so in case of
> new + destroy (which should be correct) you just call sasl_server_done
> without any call to spice_server_init.
> 
> Had a brief look at sasl_server_init/sasl_server_done the the counter
> is not updated atomically, potentially can be racy but is really
> unlikely and is not up to us to fix it.
> 

I agree with the refcount implementation, however at
https://www.cyrusimap.org/sasl/sasl/reference/manpages/library/sasl_server_init.html#std:saslman-sasl_server_init(3)
they state

"sasl_server_init() initializes SASL. It must be called before any calls to sasl_server_start, and only once per process."

Which seems to indicate the opposite.

Frediano