[L2Ork-dev] Delayed pd_unbind

[Albert Graef]

On Tue, Nov 12, 2013 at 11:34 PM, Ivica Ico Bukvic <ico at vt.edu> wrote:
> The call should guarantee traversal over every list given because all
> bindlist_* calls account for it within the same loop (below is an excerpt
> from bindlist_float, but the same should hold true for all other types as
> well):
>
> static void bindlist_float(t_bindlist *x, t_float f)
> {
>     t_bindelem *e;
>         change_bindlist_via_graph = 1;
>     for (e = x->b_list; e; e = e->e_next)
>         if (e->e_who != NULL) pd_float(e->e_who, f);
>         if (change_bindlist_via_graph > 1)
>                 bindlist_cleanup(x);
>         change_bindlist_via_graph = 0;
> }

The problem here is what may happen in the call to pd_float() (or
pd_bang(), pd_symbol(), etc.). This sends a message to a receiver
object which might in turn trigger a recursive call to one of the
bindlist functions. When this recursive call returns, it will have
reset the change_bindlist_via_graph variable to zero, and thus the
bindlist_cleanup() will never be invoked in the topmost call to
bindlist_float(). That's why a simple global flag isn't enough.

However, you can easily fix this by saving the value of
change_bindlist_via_graph in a local variable at entry and restoring
it at exit, instead of simply resetting the value to 0. That is:

static void bindlist_float(t_bindlist *x, t_float f)
{
    t_bindelem *e;
        int save = change_bindlist_via_graph;
        change_bindlist_via_graph = 1;
    for (e = x->b_list; e; e = e->e_next)
        if (e->e_who != NULL) pd_float(e->e_who, f);
        if (change_bindlist_via_graph > 1)
                bindlist_cleanup(x);
        change_bindlist_via_graph = save;
}

Unfortunately, even that doesn't cover all possible memory leaks. When
the action triggered by pd_float() (or pd_bang(), pd_symbol(), etc.)
happens to call pd_unbind() for some *other* symbol, pd_unbind() may
delete an entry in some bind list y different from x. Since
change_bindlist_via_graph is set at this point, the deletion will be
delayed, but the call to bindlist_cleanup() in bindlist_float() will
only collect the deleted entries in x, not in the other list y which
it knows nothing about, resulting in a memory leak.

I admit that this kind of use of pd_unbind() isn't all that common,
but it works fine with vanilla Pd, and in any case my pd-faust
external needs to do that kind of stuff when a Faust module gets
reloaded at runtime, in which case there's a rebinding of various
receivers related to the Faust object. This is one of the key features
of pd-faust, so I wouldn't want to do without it when using pd-l2ork.

However, if I understood you correctly, then for your purposes it
would be good enough to just delay the freeing of entries in a
specific bind list x while x is still being traversed in one of the
bindlist functions, right? Then it should be possible to add another
global variable with the active bind list so that the delaying could
be restricted to just the active bind list, making all other calls to
pd_unbind() behave like in vanilla Pd. I'll prepare a pull request
which does that and also includes the fixes discussed above.

Albert

-- 
Dr. Albert Gr"af
Dept. of Music-Informatics, University of Mainz, Germany
Email:  aggraef at gmail.com
WWW:    https://plus.google.com/+AlbertGraef