mworks / mworks-issues

MWorks issue tracker
0 stars 0 forks source link

Attached action can lead to reference cycle in Variable #443

Closed cstawarz closed 1 year ago

cstawarz commented 1 year ago

The following code creates a reference cycle that prevents the Variable instance associated with "myvar" from being deallocated:

var myvar = false {
    if (myvar) {
        // Do something
    }
}

This happens because the variable holds a strong reference to the attached "if" action, and the action holds a strong reference to "myvar" (as returned by ComponentRegistry::getVariable). Note that if the action used a more complicated expression involving "myvar" (e.g. my_var > 2), then this code would not create a reference cycle, because the expression would be stored as a ParsedExpressionVariable, which does not hold strong references to contained variables.

I think the right fix for this is

  1. add a removeNotifications method to Variable, and
  2. have VariableRegistry::reset call removeNotifications on all variables before releasing them
cstawarz commented 1 year ago

An alternative fix could be for Variable to store weak references to attached actions, since the component registry holds strong references to the actions (I think).

cstawarz commented 1 year ago

An alternative fix could be for Variable to store weak references to attached actions

More precisely, ActionVariableNotification should hold a weak reference to its action.