On Tue, 2013-06-18 at 20:45 +0300, Tanu Kaskinen wrote:
> On Tue, 2013-06-18 at 21:06 +0530, Arun Raghavan wrote:
> > On Mon, 2013-06-17 at 18:16 +0530, Arun Raghavan wrote:
> > > We need the mainloop lock to be taken around pa_mainloop_api_once() to
> > > prevent an assert due to the defer event creation and setting of the
> > > destroy callback not being performed atomically.
> > > ---
> >
> > No comments, so pushed both of these out now.
>
> I would appreciate it if you gave a bit more time to comment on patches.
> FWIW, I wait for a week on my patches before pushing due to lack of
> comments.
I think we should decrease that cycle and try to not have a long lag
between writing a patch and pushing it. The commits list provides a good
mechanism to check anyway.
> I have an issue with the changed documentation (and the commit messages,
> but it's too late to change those now). It's not really relevant that
> defer_new() and defer_set_destroy() aren't run somehow together
> ("atomically"). Even if defer_new() took a free callback as a parameter,
> making the defer_set_destroy() call unnecessary, there would still be
> the exact same rules regarding threads. If you create a defer event, it
> must always be done from the mainloop thread, if you don't know what the
> mainloop implementation is. If you do know the mainloop implementation,
> e.g. pa_threaded_mainloop, then the general rule may be relaxed, given
> that you follow the rules of the mainloop implementation, e.g. call a
> locking function first. In case of pa_threaded_mainloop, you can not
> call defer_new() without locking first.
I guess that calls for having a more generic documentation about
understanding threading restrictions of whatever mainloop is being used.
I'll write that up.
-- Arun
