Fix memory leaks

[Marckvdv]

Currently, Storm has some memory leaks. This issue tracks memory leaks and aims to fix them. I will update it accordingly. Once all leaks are fixed (or at least those by the tests), leak detection for AddressSanitizer can be turned on again (as per #197), to prevent new memory leaks from being pushed.

Leaks

• [ ] ExpressionManager and friends. Many of the variables contained within an ExpressionManager contain references to that ExpressionManager. Sometimes as a const & (as in BaseExpression), sometimes as shared_ptr (as in Type) and sometimes as const* (as in Variable). Ideally these should all be the same. The usage of the shared_ptr causes a circular dependency, which means that a ExpressionManager can never be freed.
• [ ] storm-pars/analysis/Order.h: usage of new all over the place but no delete's.
• [ ] Sylvan (wrapper?) seems to be leaking.

[tquatmann]

Regarding the ExpressionManager-pointers: I think this is what std::weak_ptr is for. However, weak_ptr is a bit slow as it creates a shared_ptr to access the element. At some point I made the state space generation 10-20% faster on some models by changing the pointer in Variable from std::weak_ptr to const*. const& doesn't do the trick everywhere since (iirc), Variable needs a default ctor. Hence, my vote is for const* unless someone comes up with a better solution with equal performance that does not require a complete overhaul of the storm::expressions.

[sjunges]

Dont we want to have a const* const?

[tquatmann]

@sjunges That would be preferable, indeed. I'm not sure if there currently is a need to change the manager somewhere in the code, though.

[Marckvdv]

I will try and replace all those references with pointers and see how far that gets me.

[Marckvdv]

It's perhaps a bit more complicated than I had hoped. In some cases, a prism::Program is created, which is the owner of its ExpressionManager. This Program is then used to construct a model, which then contains references to things contained within the ExpressionManager. The Program is then deconstructed, and the created model contains invalid references. This used to not be a problem because the circular dependencies meant that the ExpressionManager would always be alive. I suppose we could fix this by giving the constructed model ownership of the ExpressionManager (i.e. add a std::shared_ptr<ExpressionManager> to sparse::Model), but other suggestions are also welcome.

[tquatmann]

I'm not a big fan of adding a std::shared_ptr<ExpressionManager> to the built sparse::model. It feels a bit counterintuitive that e.g. a DTMC has ownership of a manager for expressions that might or might not be used somewhere. When do we actually need to access any kind of expressions after model building? I can only think of

• StateValuations
• ChoiceOrigins, and
• AtomicExpressionFormula

My proposal is to add a std::shared_ptr<ExpressionManager> to those. Iirc, ChoiceOrigins already have a shared_ptr to the PRISM/JANI object, so those are probably fine.

One could also think of adding a std::shared_ptr<ExpressionManager> to storm::expressions::Expression. I'm not sure if this has performance impacts, though.