[pulseaudio-discuss] [PATCH v5 39/39] bluetooth: Revive module-bluetooth-discover

[Tanu Kaskinen]

On Tue, 2013-10-08 at 11:19 -0300, João Paulo Rechi Vita wrote:
> On Sun, Sep 29, 2013 at 1:39 PM, Tanu Kaskinen
> <tanu.kaskinen at linux.intel.com> wrote:
> > On Tue, 2013-09-24 at 19:45 -0300, jprvita at gmail.com wrote:
> >> +void pa__done(pa_module* m) {
> >> +    struct userdata *u;
> >> +
> >> +    pa_assert(m);
> >> +
> >> +    if (!(u = m->userdata))
> >> +        return;
> >> +
> >> +    if (u->bluez5_module)
> >> +        pa_module_unload(m->core, u->bluez5_module, true);
> >> +
> >> +    if (u->bluez4_module)
> >> +        pa_module_unload(m->core, u->bluez4_module, true);
> >
> > This crashes when shutting down the daemon, because when the daemon
> > unloads all modules, module-bluez*-discover gets unloaded before
> > module-bluetooth-discover, so the y->bluez5_module and u->bluez4_module
> > pointers become stale. I see two ways of fixing this: add a hook that is
> > fired when modules are unloaded and use that hook in
> > module-bluetooth-discover to drop the reference to the unloaded module,
> > or unload module-bluetooth-discover immediately after loading
> > module-bluez5-discover and module-bluez4-discover. The second solution
> > is of course much simpler, but I proposed that already earlier, and you
> > didn't like that.
> >
> 
> It doesn't crash (and that's what I'm experiencing here) because
> pa_module_unload() will look for that module pointer in its internal
> hash of modules before trying to unload it. I agree we are left with a
> stale pointer, but as long as we don't dereference it, we should be
> fine.

pa_module_unload() dereferences the pointer already before

    if (!(m = pa_idxset_remove_by_data(c->modules, m, NULL)))
        return;

which I think you are referring to as the safeguard. The line that
crashes is this:

    if (m->core->disallow_module_loading && !force)

It doesn't crash always, because m->core is a random address that may or
may not be possible to dereference (it was 0 when I debugged this).

When this line doesn't crash, I get an assertion about pa_core.shared
not being empty when pa_core is freed. I hope that's another symptom of
this same bug (I looked at the code and the management of pa_core.shared
seemed correct).

-- 
Tanu